Summary: | Dispatch a progress event before dispatching abort, error or timeout event | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
Component: | XML | Assignee: | youenn fablet <youennf> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, commit-queue, ossy | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 120828 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
youenn fablet
2014-01-07 05:31:16 PST
Created attachment 220529 [details]
fix over 120828 patch
fix will require updating some existing http tests
Please add a changelog entry and set r? flag to ask review. Created attachment 221034 [details]
first patch
Patch only takes care of abort, error and timeout events. This behavior aligns with the XHR spec and with Blink and Firefox. It would be nice to also have a progress ProgressEvent in the normal load case (for consistency with the error cases plus it aligns with the spec). But Firefox, Blink and WPT tests consistently disagree with that behavior. Comment on attachment 221034 [details] first patch View in context: https://bugs.webkit.org/attachment.cgi?id=221034&action=review > LayoutTests/http/tests/xmlhttprequest/upload-onloadend-event-after-abort.html:-45 > - logProgressEvent(e); This doesn't look right - why don't we log this event any more? It doesn't materially affect what this test is testing for, but it could be confusing in the future. (In reply to comment #5) > (From update of attachment 221034 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221034&action=review > > > LayoutTests/http/tests/xmlhttprequest/upload-onloadend-event-after-abort.html:-45 > > - logProgressEvent(e); > > This doesn't look right - why don't we log this event any more? > > It doesn't materially affect what this test is testing for, but it could be confusing in the future. I found the test easier to read if its purpose is "check all the ProgressEvent after abort() is called.". The not-logged-anymore event is sent before abort() is called. I can revert the change if it adds more confusion than clarity. > I found the test easier to read if its purpose is "check all the ProgressEvent after abort() is called.".
OK.
Comment on attachment 221034 [details] first patch Clearing flags on attachment: 221034 Committed r161891: <http://trac.webkit.org/changeset/161891> All reviewed patches have been landed. Closing bug. Build fix in <http://trac.webkit.org/r161894>. How did this even build in release mode? Twice the same thing... Thanks for fixing it |