[webkit-reviews] review denied: [Bug 46069] [Chromium] User style layout tests don't pass on Chromium : [Attachment 68258] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 21 13:19:52 PDT 2010


David Levin <levin at chromium.org> has denied Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 46069: [Chromium] User style layout tests don't pass on Chromium
https://bugs.webkit.org/show_bug.cgi?id=46069

Attachment 68258: Patch
https://bugs.webkit.org/attachment.cgi?id=68258&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68258&action=review

I think the compile assert really should be added before committing this.

> LayoutTests/platform/chromium/drt_expectations.txt:156
> +BUG49021 : userscripts/mixed-case-stylesheet.html = PASS

Why do we have PASS lines in here as opposed to just deleting the line?

> LayoutTests/userscripts/mixed-case-stylesheet.html:-5
> -    window.layoutTestController.dumpAsText();

I think removing "window." is "clean up the test code slightly".

Ideally this would be in a different patch so one doesn't think that it has
anything to do with the fix. (I was wondering how this helped fix the issue.)

> WebKit/chromium/public/WebView.h:74
> +    };

chromium/public change = Darin Fisher review needed.

> WebKit/chromium/src/WebViewImpl.cpp:2042
> +					  
static_cast<WebCore::UserStyleInjectionTime>(injectionTime));

Please add a compile assert to ensure that the enum values continue to be the
same for both of these.


More information about the webkit-reviews mailing list