-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding note about loadstart event firing async in Fx 79 onwards #6364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, Chris.
Since the synchronous behavior is non-standard, I'd like to propose flipping this around a little. Something like:
- A plain
"version_added": "79"
- A
"version_added": true
with"partial_implementation": true
and a note saying that the event dispatched synchronously (perhaps describing it as non-standard or unlike other browsers).
What do you think?
@ddbeck Yup, I agree with you; updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Chris. I skipped over some details in the first place. A few more suggestions for you
api/FileReader.json
Outdated
}, | ||
"firefox": [ | ||
{ | ||
"version_added": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, sorry I forgot a couple things in my original comment. First, this is removed:
"version_added": true, | |
"version_removed": "79", |
api/FileReader.json
Outdated
"notes": "<code>loadstart</code> event originally dispatched synchronously (should be asynchronously as per spec)." | ||
}, | ||
{ | ||
"version_added": "79" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this entry should be before the other
api/FileReader.json
Outdated
{ | ||
"version_added": true, | ||
"partial_implementation": true, | ||
"notes": "<code>loadstart</code> event originally dispatched synchronously (should be asynchronously as per spec)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor tense thing here
"notes": "<code>loadstart</code> event originally dispatched synchronously (should be asynchronously as per spec)." | |
"notes": "<code>loadstart</code> event dispatches synchronously (should be asynchronously as per spec)." |
@ddbeck How now brown cow? ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you!
As per https://bugzilla.mozilla.org/show_bug.cgi?id=1502403