Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2266353004:
Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure (Closed)
|
Created:
4 years, 4 months ago by hayato Modified:
4 years, 3 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sofeykov, Timothy Loh Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure
Document::styleForElementIgnoringPendingStyleSheets() is wrongly using
parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead.
BUG=639776
Committed: https://crrev.com/661a03d007da761088f30689eba00acd524b3e15
Cr-Commit-Position: refs/heads/master@{#414649}
Patch Set 1 #Patch Set 2 : fix #
Total comments: 9
Patch Set 3 : update the tests #
Total comments: 1
Patch Set 4 : update the test #
Total comments: 1
Messages
Total messages: 52 (23 generated)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
fix
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Make the computation of computedStyle use LayoutTreeBuilderTraversal::parent(), instead parentOrShadowHost BUG=639776 ========== to ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNodeOrShadowHost() where LayoutTreeBuilderTraversal::parent should be used instead. BUG=639776 ==========
hayato@chromium.org changed reviewers: + kochi@chromium.org, rune@opera.com
PTAL https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html (left): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html:31: shouldBe('getColorProperty("span-child")', '"rgb(255, 0, 0)"'); This is a bug of the original test, which should be updated.
Description was changed from ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNodeOrShadowHost() where LayoutTreeBuilderTraversal::parent should be used instead. BUG=639776 ========== to ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ==========
https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html (left): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html:31: shouldBe('getColorProperty("span-child")', '"rgb(255, 0, 0)"'); On 2016/08/23 11:50:26, hayato wrote: > This is a bug of the original test, which should be updated. Is there a spec which talks about inheritance for unassigned nodes? I'm not able to find anything. https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); Here, the span gets the green color from the light-tree parent. I don't get the description to match what the test checks for.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
Looks there are failing tests. Let me fix that, and upload a new patch set. https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html (left): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html:31: shouldBe('getColorProperty("span-child")', '"rgb(255, 0, 0)"'); Good point. http://w3c.github.io/webcomponents/spec/shadow/#flattening-algorithm should determine the *parent* in CSS inheritance. (The algorithm has been upstreamed to CSS Scoping: https://drafts.csswg.org/css-scoping/#flattening) For a host's child which is not assigned to any slot, I think there is no parent for that. https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); It gets the green color from the parent of the slot. A style in light-tree has only "#host { color: red }" rule.
update the tests
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hayato@chromium.org changed reviewers: + jfernandez@igalia.com
Adding jfernandez@igalia.com to a reviewer since I have updated parse-alignment-of-root-elements.html. jfernandez@, could you check it?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/08/24 04:16:29, hayato wrote: > Adding mailto:jfernandez@igalia.com to a reviewer since I have updated > parse-alignment-of-root-elements.html. > > jfernandez@, could you check it? Before commenting on the changes for each test case, I'd like to understand better the issue and how it affects to what the Alignment specification states. "The auto keyword computes to the computed value of justify-items on the parent (minus any legacy keywords), or normal if the box has no parent." I've asked Tab to clarify what 'parent' actually means and how to interpret it in a ShadowDOM context. He replied it's indeed the element's parent defined in the HTML tree, but he didn't mention explicitly any ShadowDOM concept. This change causes that the element with id "slotted" resolves its 'auto' value using the "normal if the box has no parent" branch. It's weird, because the element has indeed a parent, but let's assume ShadowDOM changes that, so that even having a parent, we can't use it for style resolution if the element in question can't participate in a FlatTree. I wonder whether this issue should be included in the Alignment spec, or at least a reference to the ShadowDOM spec. Anyway, before the change I noticed that the parent used to resolve the style was different before and after assigning the slot. That's precisely what the test case about the slotted element was trying to verify. Probably such case doesn't make sense now.
Alignment specification should not be affected by this change. This CL affects only on how CSS property values are inherited. I guess the implementation of alignment is using CSS property inheritance mechanism internally because it is affected by this CL. If it does not use CSS property inheritance mechanism internally, this would not be affected by this CL. I have seen several instances of such attributes in Blink. Several attributes are implemented as a CSS property value internally. If *parent* should actually mean "parent in the DOM tree (instead of the flat tree)", the implementation of alignment should be wrong, I guess. However, that is an orthogonal issue of this CL, I think. I am not familiar with alignment spec. Please correct me if I miss something...
On 2016/08/24 09:45:25, hayato wrote: > Alignment specification should not be affected by this change. > > This CL affects only on how CSS property values are inherited. > I guess the implementation of alignment is using CSS property inheritance > mechanism internally because it is affected by this CL. Alignment does not use CSS property inheritance. However, we have implemented a StyleAdjuster::adjustStyleForAlignment logic which uses element's parent style to resolve some 'auto' values. For what I understand, this kind style adjustment is done for other properties as well and it usually depends on certain element's parent CSS properties. > > If it does not use CSS property inheritance mechanism internally, this would not > be affected by this CL. Isn't the StyleResolver::adjustComputedStyle logic being affected by this change, in a way ? > > If *parent* should actually mean "parent in the DOM tree (instead of the flat > tree)", the implementation of alignment should be wrong, I guess. The spec just uses the term "parent", which Tab later clarified as follows: "Parent element" refers to the element tree; it doesn't care about box-tree structure. (And so there's no layout-time determination; this behavior can instead be resolved at inheritance time.)" Our implementation relies on what the StyleResolver has as parent style. I guess ShadowDOM manages that in a different way than other layout models. > However, that is an orthogonal issue of this CL, I think. > This discussion only affects on how to define the tests; it doesn't affect to the change itself, which I guess is correct (my understanding of the ShadowDOM spec is very limited). Since alignment blindly relies on what the StyleResolver has as parent style in order to perform the style adjustments, I'd just need to understand what to expect from the shadowDOM and slot elements cases when trying to resolve the 'auto' values. Adding @timloh to the conversation, as he was involved in the definition of the StyleAdjuster logic for alignment.
Description was changed from ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ========== to ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ==========
1. "Parent element" refers to the element tree; it doesn't care about box-tree structure. (And so there's no layout-time determination; this behavior can instead be resolved at inheritance time.)" 2. Our implementation relies on what the StyleResolver has as parent style. I guess ShadowDOM manages that in a different way than other layout models. 1 and 2 are inconsistent. You can not use StyleResolver *as is* for this purpose because StyleResolver is based on the flat tree structure. It StyleResolver uses a Node::parentNode() to resolve a parent style, it is a bug, and that is actually what this CL is fixing. I am afraid that you have to give up either of 1 or 2. :(
https://codereview.chromium.org/2266353004/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html (right): https://codereview.chromium.org/2266353004/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html:215: }, "Check out how align-self uses the 'slot' element's parent (Shadow Node) as 'slotted' element' s parent after the 'slot' is assigned."); I think the test already assumes that *parent* means "parent in the flat tree", instead of "parent in the DOM tree".
On 2016/08/24 10:57:20, hayato wrote: > https://codereview.chromium.org/2266353004/diff/40001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html > (right): > > https://codereview.chromium.org/2266353004/diff/40001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html:215: > }, "Check out how align-self uses the 'slot' element's parent (Shadow Node) as > 'slotted' element' s parent after the 'slot' is assigned."); > I think the test already assumes that *parent* means "parent in the flat tree", > instead of "parent in the DOM tree". Yes, I defined the tests considering ShadowDOM as a especial case. That's why we finally decided to use the StyleResolver to resolve the 'auto' values. That way we could deal with shadowDOM and regular elements in a transparent way. I just needed to understand which was the expected behavior for this auto value resolution for shadowDOM and slotted elements. Those expectations were based on what it was implemented before this change. Now it seems expectations have to change, and probably the test itself, because I defined the different test cases considering the old behavior. Now I'm more worried about what you stated about inconsistencies between that the Alignment spec states about auto value resolution and what we finally implement. That's why I commented before whether the Alignment spec should take into account the flat tree concept.
On 2016/08/24 at 11:09:01, jfernandez wrote: > I just needed to understand which was the expected behavior for this > auto value resolution for shadowDOM and slotted elements. Those > expectations were based on what it was implemented before this > change. Now it seems expectations have to change, and probably > the test itself, because I defined the different test cases considering > the old behavior. Okay. For an unassigned node, it should be considered as "there is no parent for that". In other words, it is a *root* node. How does "document root node"'s auto value resolution work? It should be same to this case, I think.
As long as we use StyleResolver, for any purpose, that means we have decided to use "parent in the flat tree", instead of "parent in the DOM tree". You might want to update the alignment spec. Please see http://w3c.github.io/webcomponents/spec/shadow/#attributes. There are also other attributes which use "flat tree" structures to resolve its parent.
On 2016/08/24 11:18:42, hayato wrote:
> On 2016/08/24 at 11:09:01, jfernandez wrote:
>
> > I just needed to understand which was the expected behavior for this
> > auto value resolution for shadowDOM and slotted elements. Those
> > expectations were based on what it was implemented before this
> > change. Now it seems expectations have to change, and probably
> > the test itself, because I defined the different test cases considering
> > the old behavior.
>
> Okay. For an unassigned node, it should be considered as "there is no parent
for
> that". In other words, it is a *root* node.
Sorry If I'm confused about the terms; in the test cases I defined I was playing
with
2 kind of nodes, the one with id "slotted" (having a "slot" property) and the
one
created with createElement('slot'), which after named it with the value of the
"slotted"'s slot property and added to the tree we can consider it as
"assigned".
Before this patch there were a different behavior on the "slotted" element,
before
and after adding the new element. Now it seems that the "slotted" element
should be treated as a "root element" (no parent). is this right ?
On the other hand, I'm adding the 'slot" element to the "shadowNode" element,
which as created under the shadowRoot. It seems that new expected result
indicates that "slot" has no parent; is that correct ? would it be treated as
a root element as well ? I would expect some kind of parental relationship
between this 'slot' element and either "shadowNode" or the 'slotted'.
>
> How does "document root node"'s auto value resolution work? It should be same
> to this case, I think.
I think treating these special nodes as root nodes would work and, in a way,
would be
complaint with Alignment spec. My only doubt is only which of these elements
should
be treated like root nodes (from the perspective of style resolution).
I understand your confusion. :) Let me use the example.
HTML:
<shadow-host>
<child1 slot=s1></child1>
<child2 slot=s2></child2>
</shadow-host>
Then, suppose the <shadow-host>'s shadow tree is:
<div>
<slot name=s1></slot>
</div>
In this case, the flat tree would be:
<shadow-host>
<div>
<slot name=s1>
<child1 slot=s1></child1>
</slot>
</div>
</shadow-host>
.. and <child2 slot=s2></child2> would be *orphan*. That means it is a *root*
node (of a tree which consist of only <child2 slot=s2></child2>).
Additional note: To make the situation complex, due to the current limitation,
Blink does not support "slots in the flat tree" yet. Thus, the actual flat tree
would be:
<shadow-host>
<div>
<child1 slot=s1></child1>
</div>
</shadow-host>
.. and we have an *orphaned tree* other than that:
<child2 slot=s2></child2>
Ok, I understand now :) non-owner LGTM for the alignment related change. I'd still have to redefine some if the tests cases based on what we have discussed here. Thanks for that. BTW, I've asked the CSS WG about the possible changes in the Alignment spec regarding the flat-tree concept. Let's see whether they clarify the issues or change the spec somehow.
https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html (left): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/dom/shadow/distribution-update-recalcs-style.html:31: shouldBe('getColorProperty("span-child")', '"rgb(255, 0, 0)"'); On 2016/08/24 03:34:58, hayato wrote: > Good point. > > http://w3c.github.io/webcomponents/spec/shadow/#flattening-algorithm should > determine the *parent* in CSS inheritance. > (The algorithm has been upstreamed to CSS Scoping: > https://drafts.csswg.org/css-scoping/#flattening) > > For a host's child which is not assigned to any slot, I think there is no parent > for that. Then shouldn't it return the empty string for computed style like we do for nodes in detached subtrees? There was a discussion about the existence of computed style of detached trees on www-style in May. Don't think there was a conclusion for that issue, though. https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); On 2016/08/24 03:34:58, hayato wrote: > It gets the green color from the parent of the slot. > A style in light-tree has only "#host { color: red }" rule. Doesn't #span1 inherit from <div slot="s1"> ?
> Then shouldn't it return the empty string for computed style like we do for nodes in detached subtrees? There was a discussion about the existence of computed style of detached trees on www-style in May. Don't think there was a conclusion for that issue, though. I do not know much about what we should return in this case. :( As far as I know, Blink/WebKit is not good at handling style for detached nodes due to the historical reasons. I have heard that yosin@ made some efforts to make it better, but that is still incomplete. I would like to resolve this issue as a separate issue until we had a clear conclusion. In this CL, I just wanted to prevent it form inheriting the *wrong* parent. That's the bottom line, and it fixes the crash. https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); On 2016/08/24 at 12:38:44, rune wrote: > On 2016/08/24 03:34:58, hayato wrote: > > It gets the green color from the parent of the slot. > > A style in light-tree has only "#host { color: red }" rule. > > Doesn't #span1 inherit from <div slot="s1"> ? Yeah, it actually inherits from <div slot="s1"> in current Blink's implementation. <div slot="s1"> is a direct parent in the flat tree, due to the lack of support of "slots in the flat tree". However, if we support "slots in the flat tree" as the spec says, we can say "it inherits a style from the slot" here, and the result should be the same. This description just honors the spec, ignoring minor difference with the current implementation. Should we say 'An element which is assigned to a slot should inherit a style from the parent of the slot because Blink does not support slots in the flat tree' ?
https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); On 2016/08/25 02:01:19, hayato wrote: > On 2016/08/24 at 12:38:44, rune wrote: > > On 2016/08/24 03:34:58, hayato wrote: > > > It gets the green color from the parent of the slot. > > > A style in light-tree has only "#host { color: red }" rule. > > > > Doesn't #span1 inherit from <div slot="s1"> ? > > Yeah, it actually inherits from <div slot="s1"> in current Blink's > implementation. <div slot="s1"> is a direct parent in the flat tree, due to the > lack of support of "slots in the flat tree". No, my point is that <div slot="s1"> is #span1's parent in the light tree, and the flat tree, regardless of whether <slot> is in the flat tree or not. > However, if we support "slots in the flat tree" as the spec says, we can say "it > inherits a style from the slot" here, and the result should be the same. > > This description just honors the spec, ignoring minor difference with the > current implementation. > > Should we say 'An element which is assigned to a slot should inherit a style > from the parent of the slot because Blink does not support slots in the flat > tree' ? If that's what you're testing here, the assert should check the computed style of <div slot="s1">, not #span1.
On 2016/08/25 02:01:19, hayato wrote: > > Then shouldn't it return the empty string for computed style like we do for > nodes in detached subtrees? There was a discussion about the existence of > computed style of detached trees on www-style in May. Don't think there was a > conclusion for that issue, though. > > I do not know much about what we should return in this case. :( > > As far as I know, Blink/WebKit is not good at handling style for detached nodes > due to the historical reasons. I have heard that yosin@ made some efforts to > make it better, but that is still incomplete. > > I would like to resolve this issue as a separate issue until we had a clear > conclusion. > > In this CL, I just wanted to prevent it form inheriting the *wrong* parent. > That's the bottom line, and it fixes the crash. OK.
On 2016/08/25 at 07:49:39, rune wrote: > https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... > File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): > > https://codereview.chromium.org/2266353004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:38: 'An element which is assigned to a slot should inherit a style from the slot.'); > On 2016/08/25 02:01:19, hayato wrote: > > On 2016/08/24 at 12:38:44, rune wrote: > > > On 2016/08/24 03:34:58, hayato wrote: > > > > It gets the green color from the parent of the slot. > > > > A style in light-tree has only "#host { color: red }" rule. > > > > > > Doesn't #span1 inherit from <div slot="s1"> ? > > > > Yeah, it actually inherits from <div slot="s1"> in current Blink's > > implementation. <div slot="s1"> is a direct parent in the flat tree, due to the > > lack of support of "slots in the flat tree". > > No, my point is that <div slot="s1"> is #span1's parent in the light tree, and the flat tree, regardless of whether <slot> is in the flat tree or not. Ah, I understand the point finally. I am sorry that I was wrong. I thought the *div* we were mentioning is <div> (The parent of the slot) in the shadow tree. :) Since this div (span's parent) does not have any significant meaning in this test, let me rewrite the test so that it is less-confusing.
update the test
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I have updated the test. https://codereview.chromium.org/2266353004/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html (right): https://codereview.chromium.org/2266353004/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/css-style-inherit.html:25: <span id="span0" slot="unassigned"></span> I have removed confusing <div>s from the light tree.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hayato@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jfernandez@igalia.com Link to the patchset: https://codereview.chromium.org/2266353004/#ps60001 (title: "update the test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ========== to ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 ========== to ========== Make Document::styleForElementIgnoringPendingStyleSheets() be aware of a flat tree structure Document::styleForElementIgnoringPendingStyleSheets() is wrongly using parentNode(), where LayoutTreeBuilderTraversal::parent() should be used instead. BUG=639776 Committed: https://crrev.com/661a03d007da761088f30689eba00acd524b3e15 Cr-Commit-Position: refs/heads/master@{#414649} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/661a03d007da761088f30689eba00acd524b3e15 Cr-Commit-Position: refs/heads/master@{#414649}
Message was sent while issue was closed.
On 2016/08/24 12:19:47, jfernandez wrote: > Ok, I understand now :) non-owner LGTM for the alignment related change. I'd > still have to redefine some if the tests cases based on what we have discussed > here. Thanks for that. > > BTW, I've asked the CSS WG about the possible changes in the Alignment spec > regarding the flat-tree concept. Let's see whether they clarify the issues or > change the spec somehow. Just for the record, here is Tab's reply to my question regarding the "Parent element" concept and the "flat-tree": https://lists.w3.org/Archives/Public/www-style/2016Aug/0085.html |
