Removed vb_reader_support_data. Looped through reader_support_data, but incremented a flag through SAXObjectType and VB_SAXObjectType while incrementing the table.
thanks, Jefferson
On 10/8/2021 8:30 PM, Jefferson Carpenter wrote:
Removed vb_reader_support_data. Looped through reader_support_data, but incremented a flag through SAXObjectType and VB_SAXObjectType while incrementing the table.
Hey, any thoughts on this patch?
This patch is RE the email from several months ago saying that altering the parseURL behavior would be more acceptable if there were tests for the VB saxreader class. I would rather add tests and fix parseURL in the right place than just fix it for the non-VB saxreader. This is eventually to fix https://bugs.winehq.org/show_bug.cgi?id=51267
Thanks! Jefferson
On 10/20/21 7:46 PM, Jefferson Carpenter wrote:
On 10/8/2021 8:30 PM, Jefferson Carpenter wrote:
Removed vb_reader_support_data. Looped through reader_support_data, but incremented a flag through SAXObjectType and VB_SAXObjectType while incrementing the table.
Hey, any thoughts on this patch?
I still think there is no reason for it to change that much, it's still ~1400 lines diff.
This patch is RE the email from several months ago saying that altering the parseURL behavior would be more acceptable if there were tests for the VB saxreader class. I would rather add tests and fix parseURL in the right place than just fix it for the non-VB saxreader. This is eventually to fix https://bugs.winehq.org/show_bug.cgi?id=51267
Could you explain how it's crashing? Maybe we could have more targeted test for this specific problem instead.
Thanks! Jefferson
On 10/20/2021 4:50 PM, Nikolay Sivov wrote:
Could you explain how it's crashing? Maybe we could have more targeted test for this specific problem instead.
The application is passing a null url into parseURL. I did a bisect and found the commit where this broke (Wine used to null check the URL) but I've lost it. I can recover that if it would be helpful.
It might be passing a null url due to another wine bug, I'm not sure, but null checking looks like it conforms with Windows behavior, at least for early versions of msxml.
I still think there is no reason for it to change that much, it's still ~1400 lines diff.
The reason for the large patch is that there's an existing parseURL test, but it's part of a much larger saxreader test, so I figured it would be better to test the whole interface than to start duplicating individual test cases for the VB saxreader. To run the existing test cases on the VB saxreader it has to expose VB content handlers and a large patch is required to implement all of those.
I could certainly send a smaller patch that queries for IVBSAXXMLReader and runs parseURL on it, but it would be side by side with the ISAXXMLReader tests or in a different test function and maintained separately, instead of automatically running all test cases under both interfaces.
It's not that big of a difference imo, if having separate sets of test cases for the saxreader and the VB saxreader would be more in the style of the project I would be happy to do that. That is, if the code should be "unrolled" in the test that's good to know.
On 10/20/2021 10:24 PM, Jefferson Carpenter wrote:
On 10/20/2021 4:50 PM, Nikolay Sivov wrote:
Could you explain how it's crashing? Maybe we could have more targeted test for this specific problem instead.
The application is passing a null url into parseURL. I did a bisect and found the commit where this broke (Wine used to null check the URL) but I've lost it. I can recover that if it would be helpful.
Found it, by the way. The commit where this broke was e2f2ad0e81b
-
Jefferson Carpenter -
Nikolay Sivov