[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