Created attachment 422368 [details] HTML file to reproduce the issue Picture elements that are created in JavaScript and appended to the DOM don't re-evaluate the image source on resize. Instead the initial image, from when the element was attached to the DOM is shown. Changing any attribute on one of the source elements seems to trigger the browser to re-evaluate the image choice and display the correct image. Check out the attached HTML file for an isolated reproduction case.
Found another workaround: Simply adding the picture element first, before appending the source-elements, works too. ```js // Works container.appendChild(picture); picture.appendChild(source1); picture.appendChild(img); ``` vs ```js // Doesn't work picture.appendChild(source1); picture.appendChild(img); container.appendChild(picture); ``` Which leads me to believe that Safari is very picky about the order of insertion.
<rdar://problem/75359328>
Thanks for the reduced test case, Marvin. Rob, this issue looks to be a regression from https://trac.webkit.org/changeset/263345/webkit. This is because when doing things in this order: * create source element * set source element's media attribute * create picture element * append source element to picture element * insert picture element into document we never end up registering the <img> as a dynamic media query dependent image with the document. This is because we only get into selectImageSource() in response to the attribute changes, which happen before the <img> has a <picture> parent, and so we don't get to the addDynamicMediaQueryDependentImage() call. What's the purpose of checking whether a newly inserted <picture> element is in the document here: https://webkit-search.igalia.com/webkit/rev/2ba963a3b7c21a24310b76fbdfa2d72b0eefbb81/Source/WebCore/html/HTMLImageElement.cpp#387-390 If we always called selectImageSource() here regardless of whether the <picture> is already in the document, that should cause us to correctly track MQ result changes.
Created attachment 427014 [details] Patch
Comment on attachment 427014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427014&action=review > Source/WebCore/ChangeLog:3 > + Ensure dynamically inserted <picture> elements listen for MQ changes. This is only about media query? > Source/WebCore/html/HTMLPictureElement.cpp:52 > + auto insertNotificationRequest = HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree); No need to restore & return this. HTMLElement/Element::insertedIntoAncestor is guaranteed to return InsertedIntoAncestorResult::Done > Source/WebCore/html/HTMLPictureElement.cpp:53 > + sourcesChanged(); This isn't quite right if it's meant to track media query changes. In that case, what we want to do is to check insertionType.connectedToDocument. > LayoutTests/fast/picture/picture-mq-insert.html:20 > +let s1 = document.createElement('source'); Please don't use two-letter variable name like s1. Spell out source1. Also use const? > LayoutTests/fast/picture/picture-mq-insert.html:25 > +let s2 = document.createElement('source'); Ditto. > LayoutTests/fast/picture/picture-mq-insert.html:30 > +let img = document.createElement('img'); const?
Created attachment 427117 [details] Patch
WPT PR for this change: https://github.com/web-platform-tests/wpt/pull/28708 Safari fails this test currently (since we don't track the MQ change), but passes after this patch. Firefox fails as it doesn't track MQ changes. Chrome fails since while it does track MQ changes, it fails on the last assertion, which checks that removing the <img> from the <picture> re-does image source selection.
Created attachment 427119 [details] Patch
Please update the patch for trunk.
Created attachment 427219 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment on attachment 427219 [details] Patch r=me assuming WPT PR lands first.
Note that we need to wait for WPT PR to land first before we can land this.
Created attachment 427244 [details] Patch
Committed r276697 (237111@main): <https://commits.webkit.org/237111@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427244 [details].