Skip to content
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

Make <img>.complete more reliable #4934

Merged
merged 2 commits into from
Oct 15, 2019
Merged

Make <img>.complete more reliable #4934

merged 2 commits into from
Oct 15, 2019

Conversation

annevk
Copy link
Member

@annevk annevk commented Sep 25, 2019

These changes are in line with all three browsers. Chrome still diverges significantly from the standard & tests though on other aspects.

Tests: web-platform-tests/wpt#19298.

Fixes #1055, fixes #4475. #4476 remains outstanding.

  • At least two implementers are interested (and none opposed):
    • Everyone seems to implement these changes already
  • Tests are written and can be reviewed and commented upon at:
    • See above
  • Implementation bugs are filed:
    • Chrome: see comment below
    • Firefox: N/A
    • Safari: N/A

(See WHATWG Working Mode: Changes for more details.)


/embedded-content.html ( diff )

These changes are in line with all three browsers. Chrome still diverges significantly from the standard & tests though on other aspects.

Tests: web-platform-tests/wpt#19298.

Fixes #1055, fixes #4475. #4476 remains outstanding.
@annevk annevk requested review from domenic and bzbarsky September 25, 2019 13:18
annevk added a commit to web-platform-tests/wpt that referenced this pull request Sep 25, 2019
Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
source Show resolved Hide resolved
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 30, 2019
…tonly

Automatic update from web-platform-tests
HTML: update <img>.complete tests

Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
--

wpt-commits: dc2e245a955c832e88d24e6a1af5f718da5491bd
wpt-pr: 19298
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Sep 30, 2019
…tonly

Automatic update from web-platform-tests
HTML: update <img>.complete tests

Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
--

wpt-commits: dc2e245a955c832e88d24e6a1af5f718da5491bd
wpt-pr: 19298
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…tonly

Automatic update from web-platform-tests
HTML: update <img>.complete tests

Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
--

wpt-commits: dc2e245a955c832e88d24e6a1af5f718da5491bd
wpt-pr: 19298

UltraBlame original commit: c775395917748fc6bf05f54e385767b974109d4a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…tonly

Automatic update from web-platform-tests
HTML: update <img>.complete tests

Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
--

wpt-commits: dc2e245a955c832e88d24e6a1af5f718da5491bd
wpt-pr: 19298

UltraBlame original commit: c775395917748fc6bf05f54e385767b974109d4a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 5, 2019
…tonly

Automatic update from web-platform-tests
HTML: update <img>.complete tests

Align them somewhat with more modern practices and fix one test that expects img.complete to change during a while loop.

Helps with whatwg/html#1055 and issues pointed out from there.

whatwg/html#4934 has proposed HTML Standard changes.
--

wpt-commits: dc2e245a955c832e88d24e6a1af5f718da5491bd
wpt-pr: 19298

UltraBlame original commit: c775395917748fc6bf05f54e385767b974109d4a
@annevk
Copy link
Member Author

annevk commented Oct 15, 2019

Okay, comment removed, I think this is good now. I'll file a Chrome bug as the tests have landed already:

@bzbarsky
Copy link
Contributor

@annevk Sorry for the lag here... These changes are fine as far as they go, but they still don't end up matching UAs, because in the spec the pending request is completed async, off a stable state, while all UAs start returning false for complete immediately as soon as the src setter returns, as long as the image data is not synchronously available in the "list of available images" (so https://html.spec.whatwg.org/multipage/images.html#updating-the-image-data step 6.3 does not return early from updating the image data). So #4884 is still unresolved, I think.

bors-servo pushed a commit to servo/servo that referenced this pull request Nov 2, 2019
…-img-completion, r=<try>

Account for pending request state in <img>.complete

Per spec update, if the \<img\> current request is completely available or broken, `complete` is only `true` if the \<img\>'s pending request is also unavailable.

> The IDL attribute complete must return true if any of the following conditions is true:
       * Both the src attribute and the srcset attribute are omitted.
       * The srcset attribute is omitted and the src attribute's value is the empty string.
       * **The img element's current request's state is completely available and its pending request is null.**
       * **The img element's current request's state is broken and its pending request is null.**

Spec diff: https://whatpr.org/html/4934/1ab7c86...02772ea/embedded-content.html#the-img-element

#24450

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24450.
- [x] These changes do not require tests because they _should_ be covered by the automatically synced WPT associated with the spec update at whatwg/html#4934.

However, the [test updated with the spec change](https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/img.complete.html) passes before and after this PR.  Should another test be added?

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 7, 2019
…-img-completion, r=<try>

Account for pending request state in <img>.complete

Per spec update, if the \<img\> current request is completely available or broken, `complete` is only `true` if the \<img\>'s pending request is also unavailable.

> The IDL attribute complete must return true if any of the following conditions is true:
       * Both the src attribute and the srcset attribute are omitted.
       * The srcset attribute is omitted and the src attribute's value is the empty string.
       * **The img element's current request's state is completely available and its pending request is null.**
       * **The img element's current request's state is broken and its pending request is null.**

Spec diff: https://whatpr.org/html/4934/1ab7c86...02772ea/embedded-content.html#the-img-element

#24450

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24450.
- [x] These changes do not require tests because they _should_ be covered by the automatically synced WPT associated with the spec update at whatwg/html#4934.

However, the [test updated with the spec change](https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/img.complete.html) passes before and after this PR.  Should another test be added?

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 12, 2019
…-img-completion, r=<try>

Account for pending request state in <img>.complete

Per spec update, if the \<img\> current request is completely available or broken, `complete` is only `true` if the \<img\>'s pending request is also unavailable.

> The IDL attribute complete must return true if any of the following conditions is true:
       * Both the src attribute and the srcset attribute are omitted.
       * The srcset attribute is omitted and the src attribute's value is the empty string.
       * **The img element's current request's state is completely available and its pending request is null.**
       * **The img element's current request's state is broken and its pending request is null.**

Spec diff: https://whatpr.org/html/4934/1ab7c86...02772ea/embedded-content.html#the-img-element

#24450

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24450.
- [x] These changes do not require tests because they _should_ be covered by the automatically synced WPT associated with the spec update at whatwg/html#4934.

However, the [test updated with the spec change](https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/img.complete.html) passes before and after this PR.  Should another test be added?

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 12, 2019
…-img-completion, r=<try>

Account for pending request state in <img>.complete

Per spec update, if the \<img\> current request is completely available or broken, `complete` is only `true` if the \<img\>'s pending request is also unavailable.

> The IDL attribute complete must return true if any of the following conditions is true:
       * Both the src attribute and the srcset attribute are omitted.
       * The srcset attribute is omitted and the src attribute's value is the empty string.
       * **The img element's current request's state is completely available and its pending request is null.**
       * **The img element's current request's state is broken and its pending request is null.**

Spec diff: https://whatpr.org/html/4934/1ab7c86...02772ea/embedded-content.html#the-img-element

#24450

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24450.
- [x] These changes do not require tests because they _should_ be covered by the automatically synced WPT associated with the spec update at whatwg/html#4934.

However, the [test updated with the spec change](https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/img.complete.html) passes before and after this PR.  Should another test be added?

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants