REGRESSION (Safari 6 - ToT): Incorrectly assumes that RenderStyle data can be shared

• Component: CSS
• Status: RESOLVED FIXED

[Morten Stenshorne]

The RenderStyle of one element with "-webkit-logical-height" and initial writing mode is shared with a later element with the same "-webkit-logical-height" value but with non-initial writing mode.

[Morten Stenshorne]

Created attachment 194542 [details]
test case

[Morten Stenshorne]

Created attachment 196466 [details]
Patch

[Andreas Kling]

Comment on attachment 196466 [details]
Patch

How is this related to the FIXME in isCacheableInMatchedPropertiesCache?

    // FIXME: CSSPropertyWebkitWritingMode modifies state when applying to document element. We can't skip the applying by caching.

In the case you are fixing, are we really only interested in the first check (the one relating to 'element') in isCacheableInMatchedPropertiesCache:


    if (element == element->document()->documentElement() && element->document()->writingModeSetOnDocumentElement())

Or in all the ones for 'style' and 'parentStyle' as well?

[Morten Stenshorne]

I don't see how it's related at all. The "if (element == element->document()->documentElement() && element->document()->writingModeSetOnDocumentElement())" check doesn't do much for the test case, since non-default writing mode isn't set on the root. The particular check I needed for the test case is "if (style->writingMode() != RenderStyle::initialWritingMode())", but I'm sure one could create test cases that demonstrate the need for all the others as well. My idea was that, in order to figure out if two elements can share properties, you need to take a closer look at _both_ of them, not just the first one.

[WebKit Review Bot]

Comment on attachment 196466 [details]
Patch

Attachment 196466 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17444027

New failing tests:
fast/multicol/overflow-content.html

[WebKit Review Bot]

Created attachment 196474 [details]
Archive of layout-test-results from gce-cr-linux-05 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

[Morten Stenshorne]

That failure was expected; see 113781. Need that fix for it to go away.

[Build Bot]

Comment on attachment 196466 [details]
Patch

Attachment 196466 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17432262

[Morten Stenshorne]

Fixed in Blink: http://code.google.com/p/chromium/issues/detail?id=236329

Unassigning myself from this one.

[Ryosuke Niwa]

This patch has been landed in Blink: https://chromium.googlesource.com/chromium/blink/+/1a47e927dca52f4a166b52665d9ade954272c62b

[Radar WebKit Bug Importer]

<rdar://problem/14612331>

[Andreas Kling]

Comment on attachment 196466 [details]
Patch

Well r=me. The Blink version also removes the Element* argument to the function which is pretty neat.

[Alexey Proskuryakov]

Andreas, do you intend to land this patch, too?

[WebKit Commit Bot]

Comment on attachment 196466 [details]
Patch

Clearing flags on attachment: 196466

Committed r153608: <http://trac.webkit.org/changeset/153608>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

It appears that this patch broke fast/multicol/overflow-content.html:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fmulticol%2Foverflow-content.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 119425

[Morten Stenshorne]

See bug 113781.

[Morten Stenshorne]

This is now blocking bug 131809. And it still depends on the fix for bug 113781.

[Morten Stenshorne]

Created attachment 229930 [details]
Patch

[Build Bot]

Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6350183520534528

New failing tests:
fast/multicol/overflow-content.html

[Build Bot]

Created attachment 229938 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5886498851258368

New failing tests:
fast/events/ghostly-mousemoves-in-subframe.html
fast/multicol/overflow-content.html

[Build Bot]

Created attachment 229939 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5245981788471296

New failing tests:
fast/events/ghostly-mousemoves-in-subframe.html
fast/multicol/overflow-content.html

[Build Bot]

Created attachment 229942 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Dave Hyatt]

Comment on attachment 229930 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 229930 [details]
Patch

Clearing flags on attachment: 229930

Committed r167716: <http://trac.webkit.org/changeset/167716>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.