New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 783055 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

SetShadowCascadeOrder is only called when a shadow root is newly created.

Project Member Reported by hayato@chromium.org, Nov 9 2017

Issue description

It looks SetShadowCascadeOrder is not being called when an element is moved from one document to another document.
 

Comment 1 by kochi@chromium.org, Nov 9 2017

Ah, got it.  We may need to check in TreeScopeAdopter or somewhere.

Comment 2 by kochi@chromium.org, Nov 16 2017

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5329c1ae913a6fcc4e6fc5f69b47d0b926429794

commit 5329c1ae913a6fcc4e6fc5f69b47d0b926429794
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Thu Nov 16 09:08:57 2017

Call SetShadowCascadeOrder() when shadow root moves across documents.

We missed code to change cascading order mode (V0 or V1) when
a shadow root moves across documents.

The layout test checks if the cascade order is applied correctly.

To check if the element adoption caused proper side effect on
the internal cascade order and "MayContainV0Shadow" flag,
a C++ unit test is necessary instead of layout tests.

Bug:  783055 
Change-Id: Idf967d9315d11948782777454de17548181038c9
Reviewed-on: https://chromium-review.googlesource.com/771270
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517033}
[add] https://crrev.com/5329c1ae913a6fcc4e6fc5f69b47d0b926429794/third_party/WebKit/LayoutTests/shadow-dom/css-cascade-upgrade-from-another-tree.html
[modify] https://crrev.com/5329c1ae913a6fcc4e6fc5f69b47d0b926429794/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/5329c1ae913a6fcc4e6fc5f69b47d0b926429794/third_party/WebKit/Source/core/dom/TreeScopeAdopter.cpp
[modify] https://crrev.com/5329c1ae913a6fcc4e6fc5f69b47d0b926429794/third_party/WebKit/Source/core/dom/TreeScopeAdopter.h
[add] https://crrev.com/5329c1ae913a6fcc4e6fc5f69b47d0b926429794/third_party/WebKit/Source/core/dom/TreeScopeAdopterTest.cpp

Comment 4 by kochi@chromium.org, Nov 16 2017

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4

commit 7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4
Author: Hayato Ito <hayato@chromium.org>
Date: Mon May 14 10:06:02 2018

Fix TreeScopeAdopter in case of nested shadow trees moving across documents

https://crrev.com/517033 still misses a case where shadow trees are nested, which was the cause of
DCHECK [1] failure of the test [2]. This CL fixes that.

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/node.cc?q=MayContainLe&sq=package:chromium&l=773
[2] virtual/incremental-shadow-dom/media/controls/overlay-play-button-document-move.html.

Bug:  776656 , 783055 
Change-Id: I22005d1518768c1750be33a818d25230efa26b76
Reviewed-on: https://chromium-review.googlesource.com/1056927
Commit-Queue: Hayato Ito <hayato@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558243}
[modify] https://crrev.com/7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4/third_party/blink/renderer/core/dom/tree_scope_adopter.cc
[modify] https://crrev.com/7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4/third_party/blink/renderer/core/dom/tree_scope_adopter.h
[modify] https://crrev.com/7ca88b5b2405ebc3181bcb08588ff3ffd5a9e3a4/third_party/blink/renderer/core/dom/tree_scope_adopter_test.cc

Sign in to add a comment