Dynamically imported iframes except first one are not appeared if the first one is "about:blank"
Reported by
babata...@gmail.com,
Apr 12 2018
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3395.0 Safari/537.36 Steps to reproduce the problem: 1. Open attached "test.html". What is the expected behavior? Two iframes are shown. What went wrong? Only one iframe is shown. Did this work before? Yes 67.0.3372.0, 66.0.3359.81 Does this work in other browsers? Yes Chrome version: 67.0.3395.0 Channel: canary OS Version: 10.0 Flash Version: If the two iframes are swapped (the "about:blank" iframe is placed after the "hello" iframe), this issue won't occur.
,
Apr 12 2018
Bisect info: 549505 (good) - 549514 (bad) https://chromium.googlesource.com/chromium/src/+log/fa01f1ef..0bd16722?pretty=fuller Suspecting r549511 = 348d23ce26e25195a70f0be40f88d72b8fad0955 = https://crrev.com/c/955544 by adithyas@chromium.org "Skip layout if possible in Element.focus" Landed in 67.0.3394.0 A per-revision bisect would be helpful...
,
Apr 12 2018
,
Apr 13 2018
babatakao@ Thanks for the issue. Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the latest Canary 67.0.3395.0 as per the original comment. Bisect Information: =================== Good Build: 67.0.3393.0 (Revision - 549377) Bad Build : 67.0.3394.0 (Revision - 549695) Not able to execute the per-revision bisect script as Runtime error is coming up. On executing the chromium bisect script, below is the Changelog URL: https://chromium.googlesource.com/chromium/src/+log/17ff699d5dcef674814a8574f9c090079c511e9c..0bd16722c5dae2e907eca74c02ed0a03c41737d9 From the above Changelog, suspecting the below change: Reviewed-on: https://chromium-review.googlesource.com/955544 adithyas@ Please check and confirm if this issue is related to your change, else help us in assigning to the right owner. Adding ReleaseBlock-Stable as this is a recent regression. Please feel free to remove if it is not applicable. Thanks.
,
Apr 13 2018
My CL is definitely the culprit, I'm taking a look at it now.
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6 commit ca2e0e8a949157d2f0e0de7762b5307abd2d57b6 Author: Adithya Srinivasan <adithyas@chromium.org> Date: Thu Apr 26 15:47:36 2018 Invalidate style when content frame is set Layout objects for iframes are only created if content_frame_ is not null. We should be invalidating the layout tree in SetContentFrame to ensure that a layout object is created or removed for the iframe the next time UpdateStyleAndLayoutTree is called. Bug: 832029 Change-Id: I0be50825151c8bab62c8a443944fa7d277427e6e Reviewed-on: https://chromium-review.googlesource.com/1013078 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Adithya Srinivasan <adithyas@chromium.org> Cr-Commit-Position: refs/heads/master@{#554037} [add] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/WebKit/LayoutTests/fast/frames/appended-iframes-layout-expected.html [add] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/WebKit/LayoutTests/fast/frames/appended-iframes-layout.html [modify] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/blink/renderer/core/css/style_change_reason.cc [modify] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/blink/renderer/core/css/style_change_reason.h [modify] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/blink/renderer/core/html/html_frame_owner_element.cc [modify] https://crrev.com/ca2e0e8a949157d2f0e0de7762b5307abd2d57b6/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
,
Apr 30 2018
Thanks for the fix, Susan please verify. Looping to folks dealing with M67 in case there is a merge required.
,
Apr 30 2018
Thank you ligimole@. adithyas@, how is the change listed at #6 looking in canary? If it is safe to merge, pls request a merge to M67 ASAP so we can pick it up for this week M67 Beta release. Thank you.
,
Apr 30 2018
The change listed looks fine in canary so far, and I verified that the change fixed the reported issue.
,
Apr 30 2018
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 30 2018
Approving merge to M67 branch 3396 based on comment #9. Pls merge ASAP. Thank you.
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/363e0952cbdb12291c1e0bd50125badc1b59ebb2 commit 363e0952cbdb12291c1e0bd50125badc1b59ebb2 Author: Adithya Srinivasan <adithyas@chromium.org> Date: Mon Apr 30 19:56:45 2018 Invalidate style when content frame is set Layout objects for iframes are only created if content_frame_ is not null. We should be invalidating the layout tree in SetContentFrame to ensure that a layout object is created or removed for the iframe the next time UpdateStyleAndLayoutTree is called. Bug: 832029 Change-Id: I0be50825151c8bab62c8a443944fa7d277427e6e Reviewed-on: https://chromium-review.googlesource.com/1013078 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Adithya Srinivasan <adithyas@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#554037}(cherry picked from commit ca2e0e8a949157d2f0e0de7762b5307abd2d57b6) Reviewed-on: https://chromium-review.googlesource.com/1036042 Reviewed-by: Adithya Srinivasan <adithyas@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#396} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [add] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/WebKit/LayoutTests/fast/frames/appended-iframes-layout-expected.html [add] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/WebKit/LayoutTests/fast/frames/appended-iframes-layout.html [modify] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/blink/renderer/core/css/style_change_reason.cc [modify] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/blink/renderer/core/css/style_change_reason.h [modify] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/blink/renderer/core/html/html_frame_owner_element.cc [modify] https://crrev.com/363e0952cbdb12291c1e0bd50125badc1b59ebb2/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
,
Apr 30 2018
Should this be closed now that the fix has been merged?
,
Apr 30 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 Deleted