484200 - src attribute on <style> makes styles not apply

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Henri Sivonen (:hsivonen)]

Build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090310 Minefield/3.2a1pre

Steps to reproduce:
1) Load http://software.hixie.ch/utilities/js/live-dom-viewer/saved/35

Actual results:
No lime background nor green text.

Expected results:
Lime background and black text.

Additional info:
Got expected results in Opera 10, IE8 and WebKit trunk.

The code to blame is
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLStyleElement.cpp#322

Comment 1

[Henri Sivonen (:hsivonen)]

See also http://software.hixie.ch/utilities/js/live-dom-viewer/saved/36

Comment 2

[Boris Zbarsky [:bzbarsky]]

Does anyone support <style src="..."> other than us?  I'd be quite happy to rip it out.

Comment 3

[Boris Zbarsky [:bzbarsky]]

For that matter, we don't seem to do a very good job of supporting it either.

Comment 4

[Boris Zbarsky [:bzbarsky]]

And that's because we started returning the @href at some point as the url.

Since no one noticed...  I propose we nix the whole thing.  Henri, want to do that?

Comment 5

[Henri Sivonen (:hsivonen)]

Yeah, I wanted to zap the code.

Comment 6

[Johnny Stenback (:jst)]

I'd love to see this go for 1.9.2, but I'm not holding the release for this.

Comment 7

[David Zbarsky (:dzbarsky)]

Attachment: patch

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 390482 [details] [diff] [review]
patch

You need tests here (probably reftests is good).

You need to set *aURI to null.

Might as well take out the "return;" at the end there while you're in this code.

Comment 9

[Boris Zbarsky [:bzbarsky]]

For extra bonus points, switch the signature to return already_AddRefed<nsIURI> instead of using an out parameter for the URI.  Should help with avoiding the problem you ran into.  ;)

Comment 10

[David Zbarsky (:dzbarsky)]

Attachment: Patch with reftest

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 390989 [details] [diff] [review]
Patch with reftest

>+++ b/content/html/content/reftests/reftest.list
>+== 484200-1a.html 484200-1b.html

Please call those 484200-1.html and 484200-1-ref.html

>+nsHTMLLinkElement::GetStyleSheetURL(PRBool* aIsInline)
>+  return GetHrefURIForAnchors().get();

No need for the .get() here.

>+nsXMLStylesheetPI::GetStyleSheetURL(PRBool* aIsInline)
>+  nsCOMPtr<nsIURI> aURI;

Please put this declaration right before the NS_NewURI call.

r=bzbarsky with those changes.

Comment 12

[David Zbarsky (:dzbarsky)]

Attachment: Patch with tests and nits

Check in please.

Comment 13

[David Zbarsky (:dzbarsky)]

Attachment: Patch with tests

Comment 14

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/a8d700960e41