1318624 - Should not start a load of an image if it's document is not an active document

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Edgar Chen [:edgar]]

We try to start the load of an image which is in a document fragment. This seem not follow the spec, see step 1 of https://html.spec.whatwg.org/multipage/embedded-content.html#updating-the-image-data.

* Test script:
http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4675

* Expected result:
no error event.

* Actual result:
got an unexpected error event.

Comment 1

[Edgar Chen [:edgar]]

Attachment: Patch, v1

WIP patch, working on the test cases ...

Comment 2

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cb6d0c08a4d98723f99ab0867c3da47ab725c09

Comment 3

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar] from comment #0)
> We try to start the load of an image which is in a document fragment. This
> seem not follow the spec, see step 1 of
> https://html.spec.whatwg.org/multipage/embedded-content.html#updating-the-
> image-data.
> 
> * Test script:
> http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4675
> 
> * Expected result:
> no error event.
> 
> * Actual result:
> got an unexpected error event.

Test other browsers: Chrome, Safari, and Edge all have no error event fired.

Comment 4

[Edgar Chen [:edgar]]

Attachment: Should not perform a load of a image if it's document is not active document, v2

Comment 5

[Edgar Chen [:edgar]]

Attachment: Add wpt for img in non-active document, v1

Comment 6

[Edgar Chen [:edgar]]

Attachment: Add wpt for img in non-active document, v2

Comment 7

[Edgar Chen [:edgar]]

Comment on attachment 8868953 [details] [diff] [review]
Should not perform a load of a image if it's document is not active document, v2

Review of attachment 8868953 [details] [diff] [review]:
-----------------------------------------------------------------

Originally, I planned to fix this along with the non-responsive load behavior changes in bug 1076583. However, bug 1076583 is blocked since there is still some possible spec issues (in terms of web-compatibility) need to be considered. So It's better to fix this separately here.

In this patch, I add checks for non-responsive mode only, since we already do same check in http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/dom/html/HTMLImageElement.cpp#945 for the responsive mode.

Josh, may I have your review? Thank you.

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8868959 [details] [diff] [review]
Add wpt for img in non-active document, v2

Review of attachment 8868959 [details] [diff] [review]:
-----------------------------------------------------------------

I have run this test on Chrome, Safari, and Edge. All of them can pass.

Comment 9

[Josh Matthews [:jdm]]

Comment on attachment 8868959 [details] [diff] [review]
Add wpt for img in non-active document, v2

Review of attachment 8868959 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/tests/html/semantics/embedded-content/the-img-element/non-active-document.html
@@ +18,5 @@
> +    var d = p.parseFromString('<img>', 'text/html');
> +    var i = d.querySelector('img');
> +    i.onerror = t.unreached_func('got unexpected error event');
> +    i.onload = t.unreached_func('got unexpected load event');
> +    i.src = '/images/green-1x1.png';

We should add a `<img src=/images/green-1x1.png>` in this file so that any loads here would be cached.

Comment 10

[Edgar Chen [:edgar]]

Attachment: Add wpt for img in non-active document, v3, r=jdm

Addressed review comment #9. Thanks for the review, Josh.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c9166bc787d
Should not perform a load of a image if it's document is not active document; r=jdm
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ccbc9a9bfc
Add wpt for img in non-active document; r=jdm

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9c9166bc787d
https://hg.mozilla.org/mozilla-central/rev/55ccbc9a9bfc