Bug 71703 - REGRESSION(r98542): Chromium: CSS text is rendered on page
Summary: REGRESSION(r98542): Chromium: CSS text is rendered on page
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 72867
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-07 10:05 PST by Pavel Feldman
Modified: 2011-11-21 19:16 PST (History)
14 users (show)

See Also:


Attachments
downloads.html (324 bytes, text/html)
2011-11-13 04:57 PST, cbentzel
no flags Details
local_strings.js (21 bytes, application/x-javascript)
2011-11-13 04:58 PST, cbentzel
no flags Details
Patch (1.54 KB, patch)
2011-11-17 21:00 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.44 KB, patch)
2011-11-17 21:55 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.52 KB, patch)
2011-11-18 16:20 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.65 KB, patch)
2011-11-18 22:51 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.64 KB, patch)
2011-11-18 23:29 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.44 KB, patch)
2011-11-19 05:33 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.97 KB, patch)
2011-11-21 18:07 PST, David Barr
no flags Details | Formatted Diff | Diff
Patch (3.92 KB, patch)
2011-11-21 18:34 PST, David Barr
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-11-07 10:05:13 PST
Downstream issue: http://code.google.com/p/chromium/issues/detail?id=101952

Chromium Win continuous builds (release only), experience strange behavior after r98542:

1. Start Chromium Release (Win)
2. Hit Ctrl + J (brings up downloads page)

Expected: all fine
Actual: CSS text is rendered in the beginning of the page.

Regressed in: http://trac.webkit.org/changeset/98542
Comment 1 Pavel Feldman 2011-11-07 10:06:54 PST
Note that accessing document properties in the JavaScript or opening inspector fixes the rendering.
Comment 2 Antti Koivisto 2011-11-08 11:49:41 PST
I can't debug Chromium Win UI features. You should come up with an HTML test case. That should not be too hard assuming Chromium is doing sensible things (and if not then it might be a Chromium bug).
Comment 3 Eric Seidel (no email) 2011-11-08 11:51:26 PST
There isn't really much information on this bug.  Do we have any idea how this affects other ports?  We believe this is only a Chromium regression?  We're certain it started with r98542 (seems plausible)?
Comment 4 Eric Seidel (no email) 2011-11-08 11:56:37 PST
I'm confused.   http://code.google.com/p/chromium/issues/detail?id=101952 Talks about the inspector not reloading when a tab crashes.  Yet the comments in this bug talk about CSS text being rendered at the begining of a page?

Can we have a screenshot?  What's the inspector doing in Chromium that the Inspector isn't doing in Safari?  Does this affect the Safari inspector?
Comment 5 Pavel Feldman 2011-11-08 12:02:06 PST
(In reply to comment #4)
> I'm confused.   http://code.google.com/p/chromium/issues/detail?id=101952 Talks about the inspector not reloading when a tab crashes.  Yet the comments in this bug talk about CSS text being rendered at the begining of a page?
> 
> Can we have a screenshot?  What's the inspector doing in Chromium that the Inspector isn't doing in Safari?  Does this affect the Safari inspector?

That's because I posted the wrong downstream bug, sorry.

Downstream bug: 

http://code.google.com/p/chromium/issues/detail?id=102046
One of the dupes containing screenshot: http://code.google.com/p/chromium/issues/detail?id=102352
Comment 6 Pavel Feldman 2011-11-08 12:05:14 PST
(In reply to comment #3)
> There isn't really much information on this bug.  Do we have any idea how this affects other ports?  We believe this is only a Chromium regression?  We're certain it started with r98542 (seems plausible)?

I was rolling out CSS-related patches from the roll one by one:
rolled out http://trac.webkit.org/changeset/98568 - still repros
rolled out http://trac.webkit.org/changeset/98573 - still repros
rolled out http://trac.webkit.org/changeset/98542 - problem vanished.
Comment 7 Eric Seidel (no email) 2011-11-08 12:08:11 PST
(In reply to comment #6)
> (In reply to comment #3)
> > There isn't really much information on this bug.  Do we have any idea how this affects other ports?  We believe this is only a Chromium regression?  We're certain it started with r98542 (seems plausible)?
> 
> I was rolling out CSS-related patches from the roll one by one:
> rolled out http://trac.webkit.org/changeset/98568 - still repros
> rolled out http://trac.webkit.org/changeset/98573 - still repros
> rolled out http://trac.webkit.org/changeset/98542 - problem vanished.

OK.  Just so I understand, I'll restate:  You took a tip-of-tree checkout and reverted exactly r98542 and the problem went away?  Or you rolled back to r98541?  Just curious.

It seems odd that CSS text would get rendered.  Unless the behavior of the HTMLStyleElement changed?  (I haven't read Antti's change in great detail yet.)
Comment 8 Eric Seidel (no email) 2011-11-08 12:10:33 PST
Do you believe this bug requires use of the "experimental extensions API" mentioned in http://code.google.com/p/chromium/issues/detail?id=102046 to trigger?  Or could this be triggered in other scenarios?
Comment 9 Pavel Feldman 2011-11-08 12:12:47 PST
> > I was rolling out CSS-related patches from the roll one by one:
> > rolled out http://trac.webkit.org/changeset/98568 - still repros
> > rolled out http://trac.webkit.org/changeset/98573 - still repros
> > rolled out http://trac.webkit.org/changeset/98542 - problem vanished.
> 
> OK.  Just so I understand, I'll restate:  You took a tip-of-tree checkout and reverted exactly r98542 and the problem went away?  Or you rolled back to r98541?  Just curious.
> 

I took:

Chromium r107577 (http://src.chromium.org/viewvc/chrome?view=rev&revision=107577) and
WebKit r98582.

I.e. exact post-roll state.

Then I rolled out the ones above one-by-one.  The problem went away once I rolled out 98542.
Comment 10 Pavel Feldman 2011-11-08 12:13:55 PST
(In reply to comment #8)
> Do you believe this bug requires use of the "experimental extensions API" mentioned in http://code.google.com/p/chromium/issues/detail?id=102046 to trigger?  Or could this be triggered in other scenarios?

I believe it was a clean profile on the windows box. It only repros on Release, I could not repro on Debug. Also, once you touch DOM programmatically (via JS), layout was getting in place.
Comment 11 Eric Seidel (no email) 2011-11-08 12:14:16 PST
Wow.  It appears this has had at least *25* separate bug reports filed on this regression in crbugs.com in the last week?  At least thats what it seems from following the dupe links in the bug you cited in comment 6.
Comment 12 Eric Seidel (no email) 2011-11-08 12:19:59 PST
I'm confused why any css should show up as text?  That would require something giving a <style> element a renderer and laying it out like a span/div?

Some of the reports claim this is related to chrome:// urls: http://code.google.com/p/chromium/issues/detail?id=102352

What is the downloads page doing to "inject" that CSS?  Or is the downloads page some (mostly) static HTML?  How is it loaded into WebKit?
Comment 13 Eric Seidel (no email) 2011-11-08 12:21:34 PST
It seems all of the duplicates are about chrome://downloads?  What is chrome://downloads doing that is special?
Comment 14 Pavel Feldman 2011-11-08 12:30:32 PST
(In reply to comment #13)
> It seems all of the duplicates are about chrome://downloads?  What is chrome://downloads doing that is special?

It looks static to me. I did not think of saving the downloads page and opening it from file:// while I had access to the win box.
Comment 15 Eric Seidel (no email) 2011-11-08 12:45:34 PST
Is my understanding correct that this has only occurred on Chromium, only on Windows, and only on Release?
Comment 16 Eric Seidel (no email) 2011-11-08 12:47:07 PST
http://code.google.com/p/chromium/issues/detail?id=102648 contains a copy of the CSS seen on the downloads page as text.
Comment 17 Pavel Feldman 2011-11-09 07:50:30 PST
(In reply to comment #16)
> http://code.google.com/p/chromium/issues/detail?id=102648 contains a copy of the CSS seen on the downloads page as text.

So I tried downloading the chrome://downloads and loading it off file:// and off localhost:8000. In both cases, I did not observe the issue.
Comment 18 Eric Seidel (no email) 2011-11-09 11:38:28 PST
(In reply to comment #17)
> (In reply to comment #16)
> > http://code.google.com/p/chromium/issues/detail?id=102648 contains a copy of the CSS seen on the downloads page as text.
> 
> So I tried downloading the chrome://downloads and loading it off file:// and off localhost:8000. In both cases, I did not observe the issue.

I've not seen a report in that tree of bugs for any page other than chrome://downloads (or the one for the experimental insert css api).  Which on the surface suggests of a chrome-using-webkit-strangely bug, rather than a WebKit-doing-something-wrong bug.  We need more information about the reproduction before there is much to do from WebKit's side.

We could of course roll out the patch, but without better understanding, it would be impossible for Antti to ever roll it back in (since he doesn't work on Chrome). :(
Comment 19 David Barr 2011-11-09 20:43:33 PST
http://code.google.com/p/chromium/issues/detail?id=102521 Google maps fails to render on the latest build
I've traced this bug to this patch too.
Comment 20 Darin Adler 2011-11-10 11:39:19 PST
(In reply to comment #19)
> http://code.google.com/p/chromium/issues/detail?id=102521 Google maps fails to render on the latest build
> I've traced this bug to this patch too.

Is that also a Chromium-specific bug or does it affect other WebKit browsers too?
Comment 21 Mike Lawther 2011-11-10 14:50:02 PST
The maps.google.com direction icon rendering problem affects all WebKit browsers (at least, I've tested Chrome Canary and WebKit Nightly on MacOS 10.6).

David has filed a bug for this https://bugs.webkit.org/show_bug.cgi?id=71996.
Comment 22 cbentzel 2011-11-11 04:01:25 PST
I was able to reduce this down to two small static files on Chrome Canary 17.0.935.0 for Win7. I'll try to repro with a webkit nightly.

Any of the following changes make the problem go away:
  - Removing the <!DOCTYPE HTML>
  - Inlining the local_strings.js script
  - Moving the <script src> out of the head and into the body.
  - Having a body which doesn't include an anchor element
  - Removing the script which changes the <html>'s element dir attribute.

downloads.html
--------------

<!DOCTYPE HTML>
<html>
  <head>
    <style>
      .header {
        overflow: auto;
        clear: both;
      }
    </style>
    <script src="local_strings.js">
    </script>
  </head>
<body>
  <a href=""></a>
  <script>
    document.childNodes[1].setAttribute("dir", "ltr");
  </script>
</body>
</html>

local_strings.js
-----------------
function Foo() {
}
Comment 23 Antti Koivisto 2011-11-12 06:04:59 PST
Did I understand correctly that the CSS text should show up on the page when loading this test file? I don't see that happening on TOT WebKit Safari/Mac build. Could you attach the test files to remove the possibility of copy-paste errors?

Looking at the test it seems unlikely that this is a dupe of Bug 71996 (which was fixed) though it might be something similar.
Comment 24 cbentzel 2011-11-13 04:57:57 PST
Created attachment 114857 [details]
downloads.html
Comment 25 cbentzel 2011-11-13 04:58:26 PST
Created attachment 114858 [details]
local_strings.js
Comment 26 cbentzel 2011-11-13 05:00:01 PST
Comment on attachment 114857 [details]
downloads.html

Two static files that reproduce the problem on Win7 Chrome 17.0.937.0

The text in the CSS file is visible in the document when viewed. Reloads make it go away.
Comment 27 cbentzel 2011-11-13 05:01:23 PST
Here are the two files which reproduce the problem on Chrome Win7 17.0.937.0

The text inside the <style> section is visible on the page. Reloads make it go away.
Comment 28 Yair Yogev 2011-11-13 09:37:16 PST
From the chromium bugtracker, by Satoshi.Matsuzaki:
The issue on chrome://downloads and chrome://history2 was resolved on Linux,
after WebKit changeset 99873. Confirmed with local custom DEPS builds.
  Chromum 109521 + WebKit 99872: FAIL
  Chromum 109521 + WebKit 99873: OK
But the issue is still happening on Windows version, even on ToT (109826).
Comment 29 Eric Seidel (no email) 2011-11-13 11:20:07 PST
(In reply to comment #28)
> From the chromium bugtracker, by Satoshi.Matsuzaki:
> The issue on chrome://downloads and chrome://history2 was resolved on Linux,
> after WebKit changeset 99873. Confirmed with local custom DEPS builds.
>   Chromum 109521 + WebKit 99872: FAIL
>   Chromum 109521 + WebKit 99873: OK
> But the issue is still happening on Windows version, even on ToT (109826).

That sounds like a possible build dependency issue. :)
Comment 30 Yair Yogev 2011-11-13 11:46:37 PST
Since so far only Chrome was tested here, i tried TOT WebKit with Windows Safari:
The CSS code was visible at the first time i opened the file (but in further attempts it wasn't, at least not without reseting the cache...but it seems the r99984 build i downloaded is really not stable in general)
Comment 31 Antti Koivisto 2011-11-14 07:00:40 PST
I can't repro (I only have OSX dev environment) and can't find any obvious reason with code inspection. Very likely it is something to do with explicitly setting the dir attribute. That involves some messy code.

If you can repro, it should be relatively easy to debug. The likely way the content of <style> would become visible is that the display attribute value for the element (RenderStyle computed by CSSStyleSelector::styleForElement) gets miscalculated or clobbered somehow. Since the test case is small you can pretty much step through the entire process to see where it goes wrong.
Comment 32 Antti Koivisto 2011-11-14 07:03:26 PST
I mean "display property". The default style sheet has "style { display: none }" so that needs to get overriden somehow.
Comment 33 cbentzel 2011-11-14 07:17:06 PST
Unable to repro on Win7 WebKit nightly r100095 (built 14 November 2011). This may be a Chromium specific issue.
Comment 34 Yair Yogev 2011-11-14 07:44:30 PST
(In reply to comment #33)
> Unable to repro on Win7 WebKit nightly r100095 (built 14 November 2011). This may be a Chromium specific issue.

i did repro it in Safari (comment #30), will try a newer build of WebKit later today...
Comment 35 Yair Yogev 2011-11-14 14:10:02 PST
weird:
i tried again with Windows Safari using WebKit r100145
1. On first attempt the CSS wasn't visible
2. After refreshing the page "Windows Problem Reporting" (werfault.exe) asked my firewall to connect the network (something crashed behind the scenes) and the CSS code showed up in the refreshed page.

it seems WebKit2WebProcess.exe keeps crashing when i try to surf using the latest WebKit builds on Windows (not just in this testcase)
Comment 36 David Barr 2011-11-17 21:00:46 PST
Created attachment 115731 [details]
Patch
Comment 37 Eric Seidel (no email) 2011-11-17 21:04:36 PST
Comment on attachment 115731 [details]
Patch

How do we test this?
Comment 38 David Barr 2011-11-17 21:23:39 PST
Actually, I just verified that this can be reproduced with DumpRenderTree.exe with the attached test case minus the external script.
Comment 39 David Barr 2011-11-17 21:37:03 PST
I get the following with Chromium Windows DumpRenderTree without the patch:
layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x28
  RenderBlock {HTML} at (0,0) size 800x28
    RenderBlock (anonymous) at (0,0) size 800x0
      RenderInline {HEAD} at (0,0) size 0x0
        RenderText {#text} at (0,0) size 0x0
    RenderBlock (anonymous) at (0,0) size 800x20
      RenderBlock {STYLE} at (0,0) size 800x20
        RenderText {#text} at (0,0) size 233x19
          text run at (0,0) width 60: ".header { "
          text run at (60,0) width 94: "overflow: auto; "
          text run at (154,0) width 71: "clear: both; "
          text run at (225,0) width 8: "}"
    RenderBlock (anonymous) at (0,20) size 800x0
      RenderInline {HEAD} at (0,0) size 0x0
      RenderText {#text} at (0,0) size 0x0
    RenderBody {BODY} at (8,28) size 784x0
      RenderInline {A} at (0,0) size 0x0 [color=#551A8B]
      RenderText {#text} at (0,0) size 0x0
      RenderText {#text} at (0,0) size 0x0

With the patch I get (also without it on other platforms):
layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x8
  RenderBlock {HTML} at (0,0) size 800x8
    RenderBody {BODY} at (8,8) size 784x0
      RenderInline {A} at (0,0) size 0x0 [color=#551A8B]
      RenderText {#text} at (0,0) size 0x0
      RenderText {#text} at (0,0) size 0x0
Comment 40 David Barr 2011-11-17 21:55:50 PST
Created attachment 115738 [details]
Patch
Comment 41 Eric Seidel (no email) 2011-11-17 22:06:23 PST
Comment on attachment 115738 [details]
Patch

Can we make this test more clear?  Maybe by use of getElementById('target') or whatever node childNodes[1] is addressing?
Comment 42 David Barr 2011-11-17 22:17:17 PST
Thanks Noel for talking through the bug with me. It really helped to cement my understanding of the problem.
Comment 43 David Barr 2011-11-17 22:47:47 PST
(In reply to comment #41)
> (From update of attachment 115738 [details])
> Can we make this test more clear?  Maybe by use of getElementById('target') or whatever node childNodes[1] is addressing?

Yes, we could say documentElement instead of childNodes[1].
Comment 44 David Barr 2011-11-17 22:50:17 PST
Comment on attachment 115738 [details]
Patch

I'll wait for the review comments to accumulate before resubmitting the patch.
Comment 45 Luke Macpherson 2011-11-17 23:02:24 PST
Comment on attachment 115738 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115738&action=review

> Source/WebCore/css/CSSStyleSelector.cpp:1268
>      ensureDefaultStyleSheetsForElement(element);

I think it would be clearer to either:
1) Make ensureDefualtStyleSheetsForElement return a boolean indicating that the cache needs to be invalidated because the default style has changed, or
2) to use a setter that automatically clears the cache when called.
As it is this code is essentially watching to see if a side-effect has occurred, which makes the intent less clear than it could be.

Another solution might be something like:
if (simpleDefaultStyleSheet && !elementCanUseSimpleDefaultStyle(element))
    return s_styleNotYetAvailable;
(Not 100% sure on that, but Antti should know.)
Comment 46 Darin Adler 2011-11-18 10:26:00 PST
Comment on attachment 115738 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115738&action=review

> Source/WebCore/css/CSSStyleSelector.cpp:1270
> +    if (wasSimpleDefaultStyleSheet && defaultStyle)
> +        m_matchStyleDeclarationCache.clear();

This code change won’t completely fix the problem. That’s because when loadFullDefaultStyle is called, *all* CSSStyleSelector objects would need to have their match style declarations caches cleared. Not just the one that triggered the call to loadFullDefaultStyle.

Not sure the best way to structure the code to make that happen, though.
Comment 47 Darin Adler 2011-11-18 10:26:57 PST
Great work on the diagnosis, by the way!
Comment 48 Antti Koivisto 2011-11-18 14:35:45 PST
Thanks for debugging, I should have figured it out from the symptoms. I think we should just disable the cache until we have full default stylesheet loaded. It is not really useful with very simple style anyway.
Comment 49 David Barr 2011-11-18 15:55:14 PST
(In reply to comment #48)
> Thanks for debugging, I should have figured it out from the symptoms. I think we should just disable the cache until we have full default stylesheet loaded. It is not really useful with very simple style anyway.

Thanks for the feedback, the alternate approach is much cleaner.
Comment 50 David Barr 2011-11-18 16:20:16 PST
Created attachment 115905 [details]
Patch
Comment 51 David Barr 2011-11-18 16:41:56 PST
Comment on attachment 115905 [details]
Patch

Just tested the alternate patch with Chrome Win and it fails the test in the browser although DumpRenderTree passes. :'(
Comment 52 Antti Koivisto 2011-11-18 17:04:47 PST
That won't work, the correct test is !simpleDefaultStyleSheet

It would be better to treat it as non-cacheable already here (setting hash to zero) so you don't even try a lookup.

2283void CSSStyleSelector::applyMatchedDeclarations(const MatchResult& matchResult)
2284{
2285    unsigned cacheHash = matchResult.isCacheable ? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;
Comment 53 David Barr 2011-11-18 22:51:18 PST
Created attachment 115932 [details]
Patch
Comment 54 David Barr 2011-11-18 22:53:08 PST
(In reply to comment #52)
> That won't work, the correct test is !simpleDefaultStyleSheet
> 
> It would be better to treat it as non-cacheable already here (setting hash to zero) so you don't even try a lookup.

Thank you Antti, your comments were spot on. I just uploaded a new patch that actually passes. :)
Comment 55 David Barr 2011-11-18 22:54:07 PST
Comment on attachment 115932 [details]
Patch

I think Antti and I are in agreement on the solution now.
Comment 56 David Barr 2011-11-18 23:29:35 PST
Created attachment 115934 [details]
Patch
Comment 57 David Barr 2011-11-18 23:30:23 PST
Comment on attachment 115934 [details]
Patch

Sorry for the noise, I neglected to address a few nits in the last patch.
Comment 58 Antti Koivisto 2011-11-19 04:41:22 PST
Comment on attachment 115934 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115934&action=review

r=me, please keep an eye on that the test actually passes on all platforms when landing.

> Source/WebCore/css/CSSStyleSelector.cpp:2285
>  void CSSStyleSelector::applyMatchedDeclarations(const MatchResult& matchResult)
>  {
> -    unsigned cacheHash = matchResult.isCacheable ? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;
> +    unsigned cacheHash = !simpleDefaultStyleSheet && matchResult.isCacheable ? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;

The ternary statement is getting bit ugly. You could just set isCacheable to false beforehand, that reads better too.

> LayoutTests/fast/css/style-tag-display-none-expected.txt:8
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x8
> +  RenderBlock {HTML} at (0,0) size 800x8
> +    RenderBody {BODY} at (8,8) size 784x0
> +      RenderInline {A} at (0,0) size 0x0 [color=#551A8B]
> +      RenderText {#text} at (0,0) size 0x0
> +      RenderText {#text} at (0,0) size 0x0

Not sure if this render tree dump is actually valid on all platforms.
Comment 59 Antti Koivisto 2011-11-19 04:48:20 PST
(In reply to comment #58)
>The ternary statement is getting bit ugly. You could just set isCacheable to false beforehand, that reads better too.

The most stylish place to do this is in CSSStyleSelector::matchUARules.
Comment 60 David Barr 2011-11-19 05:33:08 PST
Created attachment 115947 [details]
Patch
Comment 61 David Barr 2011-11-19 05:48:26 PST
Comment on attachment 115947 [details]
Patch

I've confirmed that this patch resolves the issue with Chromium Win.
Antti, thanks again for the feedback, correctness and elegance need not be mutually exclusive.
Comment 62 WebKit Review Bot 2011-11-19 07:35:14 PST
Comment on attachment 115947 [details]
Patch

Clearing flags on attachment: 115947

Committed r100856: <http://trac.webkit.org/changeset/100856>
Comment 63 WebKit Review Bot 2011-11-19 07:35:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 64 Steve Block 2011-11-21 03:19:25 PST
Rolled out in http://trac.webkit.org/changeset/100896
Comment 65 Eric Seidel (no email) 2011-11-21 11:41:10 PST
(In reply to comment #64)
> Rolled out in http://trac.webkit.org/changeset/100896

But why?  What did it break and how?  Are we sure the test didn't just need a rebaseline?
Comment 66 David Barr 2011-11-21 18:07:54 PST
Created attachment 116160 [details]
Patch
Comment 67 Dimitri Glazkov (Google) 2011-11-21 18:14:51 PST
Comment on attachment 116160 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116160&action=review

> LayoutTests/ChangeLog:10
> +        * platform/chromium/fast/css/style-tag-display-none-expected.png: Added.

This should be in fast/css, since it'll be the same result for all platforms.
Comment 68 David Barr 2011-11-21 18:34:08 PST
Created attachment 116165 [details]
Patch
Comment 69 WebKit Review Bot 2011-11-21 19:16:06 PST
Comment on attachment 116165 [details]
Patch

Clearing flags on attachment: 116165

Committed r100976: <http://trac.webkit.org/changeset/100976>
Comment 70 WebKit Review Bot 2011-11-21 19:16:15 PST
All reviewed patches have been landed.  Closing bug.