Skip to content

/ servo

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

Jump to bottom

Update img.complete implementation to match spec #24450

Open
jdm opened this issue Oct 15, 2019 · 2 comments · May be fixed by #24462
Open

Update img.complete implementation to match spec #24450

jdm opened this issue Oct 15, 2019 · 2 comments · May be fixed by #24462
Labels
A-content/dom A-content/images C-assigned I-spec-update

Comments

@jdm
Copy link
Member

@jdm jdm commented Oct 15, 2019

whatwg/html#4934
@twilco
Copy link

@twilco twilco commented Oct 16, 2019

I'll take a crack at this! @highfive: assign me?
@highfive highfive added the C-assigned label Oct 16, 2019
@highfive
Copy link

@highfive highfive commented Oct 16, 2019

Hey @twilco! Thanks for your interest in working on this issue. It's now assigned to you!
twilco added a commit to twilco/servo that referenced this issue Oct 16, 2019
@twilco
Account for pending request state in <img>.complete
e8fb883 
Per spec update, if the current request is completely available or broken, <img>.complete is only true if the pending request is unavailable.

Issue servo#24450
@twilco twilco linked a pull request that will close this issue Oct 16, 2019
Open
4 of 4 tasks complete
twilco added a commit to twilco/servo that referenced this issue Oct 29, 2019
@twilco
Properly represent null-state of pending request
960c9d2 
The <img> spec frequently mentions setting and checking the
null-status of pending request, but prior to this commit we did not have
a consistent way to represent this.  This commit changes the
`pending_request` to be an DomRefCell<Option<ImageRequest>>, where a None value corresponds to a
null pending request in the specification.

<img>.complete is now only true if pending request is null (None).

Issue servo#24450
bors-servo added a commit that referenced this issue Nov 2, 2019
@bors-servo
Auto merge of #24462 - twilco:issue-24450/factor-pending-request-into…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
652bc74 
…-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 added a commit that referenced this issue Nov 7, 2019
@bors-servo
Auto merge of #24462 - twilco:issue-24450/factor-pending-request-into…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
54a4a26 
…-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 added a commit that referenced this issue Nov 12, 2019
@bors-servo
Auto merge of #24462 - twilco:issue-24450/factor-pending-request-into…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5a38178 
…-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 added a commit that referenced this issue Nov 12, 2019
@bors-servo
Auto merge of #24462 - twilco:issue-24450/factor-pending-request-into…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
74f4bda 
…-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
Assignees
No one assigned
Labels
A-content/dom A-content/images C-assigned I-spec-update
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Account for pending request state in <img>.complete
3 participants
@jdm @highfive @twilco
You can’t perform that action at this time.