Bug 15242 – Needs test: Improperly Linked CSS document (404) still loads contained styles

• Component: Tools / Tests
• Status: ASSIGNED

[Peter Peterson]

Observed behavior:
A CSS file is included on a page with an invalid path. A 404 html page is given in response by the server. The CSS styles on the 404 html page will be used on the original page.

Expected behavior: 
The contents of the returned 404 page are ignored.

[Peter Peterson]

Created attachment 16329 [details]
Example of improper loading of css styles from 404 document

A 404 page is loaded as a CSS document. 

Observed behavior: 
"Normal" link is blue (browser default), "u link" is green, "l link" is grey

Expected behavior:
All links are the browser default color

[Alexey Proskuryakov]

Confirmed with r25667 (and with shipping Safari/WebKit).

[Mark Rowe (bdash)]

<rdar://problem/5494250>

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> Created an attachment (id=16329) [edit]
> Example of improper loading of css styles from 404 document
> 
> A 404 page is loaded as a CSS document. 
> 
> Observed behavior: 
> "Normal" link is blue (browser default), "u link" is green, "l link" is grey

Note that this test only works if http://webkit.org/ is not in your browser history!  You'll either need to change the "http://webkit.org/" URL in the test to a URL that isn't in your browser history, or you need to clear your browser history through "Reset Safari" in the "Safari" menu.

(In reply to comment #4)
> You'll either need to change the "http://webkit.org/" URL in the test
> to a URL that isn't in your browser history, or you need to clear your browser
> history through "Reset Safari" in the "Safari" menu.

You can delete individual history items in Safari's bookmarks viewer.

[Mark Larson (Google)]

Created attachment 18035 [details]
Another test case for interpreting styles in a 404 page

I confirmed this on Windows (Safari 3.0.4 523/13). I created a test case before finding this bug already filed. I've attached it FWIW (it uses a span.nav instead of an A, so it doesn't depend on your visited link history).

Curiously, the body{} style in the 404 response does not seem to get applied. Google's 404 reponse contains

<style><!--
body {font-family: arial,sans-serif}
div.nav {margin-top: 1ex}
div.nav A {font-size: 10pt; font-family: arial,sans-serif}
span.nav {font-size: 10pt; font-family: arial,sans-serif; font-weight: bold}
div.nav A,span.big {font-size: 12pt; color: #0000cc}
div.nav A {font-size: 10pt; color: black}
A.l:link {color: #6f6f6f}
A.u:link {color: green}
//--></style>

I haven't tested all the rules, but I've confirmed that span.nav gets applied but body does not.

[Dave Hyatt]

Created attachment 18920 [details]
Patch that fixes this bug

Not really sure if it is a good way to do it or not.  I just stop the feeding of data to the CachedResource when a 404 happens.

Should probably apply to other 4xx and 5xx codes.

[Darin Adler]

Comment on attachment 18920 [details]
Patch that fixes this bug

Why handle 404 different from other 4xx errors? What about 3xx and 5xx?

[Darin Adler]

Comment on attachment 18920 [details]
Patch that fixes this bug

I think the approach in this patch is OK, but it should be for all 4xx and 5xx, not specifically 404.

I also think that instead of calling data() and finish(), you should call error().

[Robert Blaut]

I've tested the bug in Webkit r34382 on OS X and, to my surprise, the issue is not replicable in this build. This test https://bugs.webkit.org/attachment.cgi?id=18035 passes in the Webkit but still fails in stock Safari 3.1.1 for OS X. 

Any confirmations?

[Alexey Proskuryakov]

I also cannot reproduce with this nightly. I think that it would make sense to isolate the revision that fixed this, and possibly to land a test case, if there isn't one already.

I think this was fixed by Hyatt for Acid3.

[Robert Blaut]

(In reply to comment #13)
> I think this was fixed by Hyatt for Acid3.
> 

Indeed, it was fixed in http://trac.webkit.org/changeset/30438 as part of fixing a bug: https://bugs.webkit.org/show_bug.cgi?id=16760

For LayoutTests I suggest to import these tests: http://biesi.damowmow.com/object/ 

[Andrew Kulinich]

*** Bug 25326 has been marked as a duplicate of this bug. ***

[Ahmad Saleem]

This started passing in 2008 and also from Comment 14, we only have this copy available:

https://web.archive.org/web/20050924094554/http://biesi.damowmow.com:80/object/002.html

Do we still want to add these test or they might be already covered one?