After <http://trac.webkit.org/changeset/180020>, we no longer have a quirks mode exception for CSS MIME types. This means that we'll start rejecting stylesheets that were previously accepted due to this quirk. I think we should log a console message in this case to help Web developers understand why their stylesheet is being rejected.
Created attachment 250847 [details] Patch
Comment on attachment 250847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250847&action=review > LayoutTests/http/tests/inspector/css/bad-mime-type-expected.txt:6 > +INFO: Unknown issue source:,security Joe, any idea why this is happening? This looks like a pre-existing issue. It looks like IssueMessage.js does not handle "security" as source. Also, the leading comma looks really weird.
Created attachment 250852 [details] Patch
Comment on attachment 250852 [details] Patch Attachment 250852 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4950895901016064 New failing tests: http/tests/security/cross-origin-css.html
Created attachment 250861 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 250852 [details] Patch Attachment 250852 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6565140031864832 New failing tests: http/tests/security/cross-origin-css.html
Created attachment 250862 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
> Joe, any idea why this is happening? This looks like a pre-existing issue. > It looks like IssueMessage.js does not handle "security" as source. Also, > the leading comma looks really weird. Yep, I know why this is happening and I'll address it right now twofold: <https://webkit.org/b/143801> Web Inspector: InspectorTest frontend console methods redirected to the frontend are wrong <https://webkit.org/b/143803> Web Inspector: Handle all possible Console message Source types in IssueMessage
Created attachment 250873 [details] Patch
Created attachment 250884 [details] Patch
Comment on attachment 250884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250884&action=review r=me > Source/WebCore/ChangeLog:11 > + previously accepted due to this quirk. I think we should log a console > + message in this case to help Web developers understand why their In the ChangeLog you can drop the "I think we should" phrasing. That is what this change is doing! You can just state it! In case this happens, we log a console message to help web developers... > Source/WebCore/css/StyleSheetContents.cpp:288 > -void StyleSheetContents::parseAuthorStyleSheet(const CachedCSSStyleSheet* cachedStyleSheet, const SecurityOrigin* securityOrigin) > +void StyleSheetContents::parseAuthorStyleSheet(const CachedCSSStyleSheet* cachedStyleSheet, const SecurityOrigin*) Since we no longer use this securityOrigin parameter we should get rid of it in the signature and at all the call sites. And I believe that means you can remove the #include as well. > LayoutTests/http/tests/security/cross-origin-css-4.html:13 > +/* Check that data: is still allowed for non-CORS cross-origin image fetches. */ Perhaps these comments could just be <p> included in the output? Would make it much more obvious if the test fails what it was trying to test without having to find the corresponding html file. > LayoutTests/http/tests/security/cross-origin-css-4.html:24 > +var onloadTest = async_test("Testing cross-origin and MIME behavior for CSS."); For instance, replacing or extending this line with those details.
Created attachment 250888 [details] Patch
Comment on attachment 250888 [details] Patch Clearing flags on attachment: 250888 Committed r182877: <http://trac.webkit.org/changeset/182877>
All reviewed patches have been landed. Closing bug.
These new tests are flaky, e.g.: https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r182877%20(3549)/http/tests/security/cross-origin-css-1-diff.txt -CONSOLE MESSAGE: Did not parse stylesheet at 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/security/resources/xorigincss1.html' because its MIME type was invalid. +CONSOLE MESSAGE: line 9: Did not parse stylesheet at 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/security/resources/xorigincss1.html' because its MIME type was invalid. I thought that the inconsistent line number detection was fixed for good, but apparently not :(
(In reply to comment #15) > These new tests are flaky, e.g.: > https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/ > r182877%20(3549)/http/tests/security/cross-origin-css-1-diff.txt > > -CONSOLE MESSAGE: Did not parse stylesheet at > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > security/resources/xorigincss1.html' because its MIME type was invalid. > +CONSOLE MESSAGE: line 9: Did not parse stylesheet at > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > security/resources/xorigincss1.html' because its MIME type was invalid. > > I thought that the inconsistent line number detection was fixed for good, > but apparently not :( :'( I'll see if I can reproduce.
(In reply to comment #16) > (In reply to comment #15) > > These new tests are flaky, e.g.: > > https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/ > > r182877%20(3549)/http/tests/security/cross-origin-css-1-diff.txt > > > > -CONSOLE MESSAGE: Did not parse stylesheet at > > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > > security/resources/xorigincss1.html' because its MIME type was invalid. > > +CONSOLE MESSAGE: line 9: Did not parse stylesheet at > > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > > security/resources/xorigincss1.html' because its MIME type was invalid. > > > > I thought that the inconsistent line number detection was fixed for good, > > but apparently not :( > > :'( I'll see if I can reproduce. I cannot reproduce. However, it looks like the relevant code is in getParserLocationForConsoleMessage() from PageConsoleClient.cpp. It looks like a line number is computed if we're still parsing the HTML document when we receive the CSS resource and try to parse it. For the CSS to get loaded while we're still parsing the link element, it has to mean that the CSS resource was in the memory cache when requested. As a result, my proposal would be to clear the memory cache using internals.clearMemoryCache() at the beginning of these tests.
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > These new tests are flaky, e.g.: > > > https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/ > > > r182877%20(3549)/http/tests/security/cross-origin-css-1-diff.txt > > > > > > -CONSOLE MESSAGE: Did not parse stylesheet at > > > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > > > security/resources/xorigincss1.html' because its MIME type was invalid. > > > +CONSOLE MESSAGE: line 9: Did not parse stylesheet at > > > 'http://127.0.0.1:8000/resources/redirect.php?url=http://localhost:8000/ > > > security/resources/xorigincss1.html' because its MIME type was invalid. > > > > > > I thought that the inconsistent line number detection was fixed for good, > > > but apparently not :( > > > > :'( I'll see if I can reproduce. > > I cannot reproduce. However, it looks like the relevant code is in > getParserLocationForConsoleMessage() from PageConsoleClient.cpp. It looks > like a line number is computed if we're still parsing the HTML document when > we receive the CSS resource and try to parse it. > > For the CSS to get loaded while we're still parsing the link element, it has > to mean that the CSS resource was in the memory cache when requested. As a > result, my proposal would be to clear the memory cache using > internals.clearMemoryCache() at the beginning of these tests. Actually, if I call "internals.clearMemoryCache();" at the beginning of these tests, the line number then shows up in the console messages for me locally. So it seems I am misunderstanding something.
Reopening due to flaky tests.
Created attachment 250906 [details] Patch
(In reply to comment #20) > Created attachment 250906 [details] > Patch Proposing a patch which gets rid of the line numbers in the CONSOLE message entirely. I don't think the line number is very useful in this case anyway and the stylesheet URL is provided and the issue is with its MIME type.
Comment on attachment 250906 [details] Patch Clearing flags on attachment: 250906 Committed r182881: <http://trac.webkit.org/changeset/182881>
(In reply to comment #22) > Comment on attachment 250906 [details] > Patch > > Clearing flags on attachment: 250906 > > Committed r182881: <http://trac.webkit.org/changeset/182881> It broke the Apple Windows build as the EWS signaled.
(In reply to comment #24) > (In reply to comment #22) > > Comment on attachment 250906 [details] > > Patch > > > > Clearing flags on attachment: 250906 > > > > Committed r182881: <http://trac.webkit.org/changeset/182881> > > It broke the Apple Windows build as the EWS signaled. Well, EWS signaled a bit late. <Fixed in https://trac.webkit.org/r182887>.