[webkit-reviews] review requested: [Bug 13596] Implement
.onprogress handler on XMLHttpRequest objects to support
progressive download content length information : [Attachment
20722] Updated with Ap's latest comments
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 21 06:41:52 PDT 2008
Julien Chaffraix <julien.chaffraix at gmail.com> has asked for review:
Bug 13596: Implement .onprogress handler on XMLHttpRequest objects to support
progressive download content length information
http://bugs.webkit.org/show_bug.cgi?id=13596
Attachment 20722: Updated with Ap's latest comments
http://bugs.webkit.org/attachment.cgi?id=20722&action=edit
------- Additional Comments from Julien Chaffraix <julien.chaffraix at gmail.com>
> I think that we need to support adding handlers via addEventListener, not
just
> .onprogress.
As mentioned on IRC, we can can register those handlers but they were not
called because of an error on my side (forgotten to call dispatchEvent). Should
be corrected.
> I have noticed that the spec actually describes when the event should be
fired:
>"every 350ms (+-200ms) or for every byte received, whichever is least
> frequent." I think it's OK to land a first version as is, to improve on this
> later.
I had not seen that.
> + // XmlHttpRequest 2 extension
> Should be "XMLHttpRequest".
Corrected.
> What is wrong with returning m_loaded?
I had to check Firefox behaviour first: there was some mention of giving
special value if computableLength was false and I thought it could apply here
too. I have check and we match Firefox so removed the 2 comments.
> Should be progressEvent to avoid constructing an AtomicString from "progress"
> again and again.
Corrected too.
I only included a "proof of concept" test case and some of your comments ask
for more of them. I will bundle more and have them landed later to have a
broader regression test suite (the patch is big enough without them ;)).
More information about the webkit-reviews
mailing list