src attribute on <style> makes styles not apply

RESOLVED FIXED

Status

()

Core
DOM
--
trivial
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: hsivonen, Assigned: dzbarsky)

Tracking

Trunk
Points:
---
Bug Flags:
wanted-next +
blocking1.9.2 -
wanted1.9.2 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Comment 1

8 years ago
See also http://software.hixie.ch/utilities/js/live-dom-viewer/saved/36
Does anyone support <style src="..."> other than us?  I'd be quite happy to rip it out.
For that matter, we don't seem to do a very good job of supporting it either.
Flags: blocking1.9.2?
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?
(Reporter)

Comment 5

8 years ago
Yeah, I wanted to zap the code.
Assignee: nobody → hsivonen

Updated

8 years ago
Component: Content → DOM
QA Contact: content → general
I'd love to see this go for 1.9.2, but I'm not holding the release for this.
Flags: wanted1.9.2+
Flags: wanted-next+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
(Assignee)

Comment 7

8 years ago
Created attachment 390482 [details] [diff] [review]
patch
(Assignee)

Updated

8 years ago
Attachment #390482 - Flags: superreview?(bzbarsky)
Attachment #390482 - Flags: review?(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.
Attachment #390482 - Flags: superreview?(bzbarsky)
Attachment #390482 - Flags: superreview-
Attachment #390482 - Flags: review?(bzbarsky)
Attachment #390482 - Flags: review-
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.  ;)
(Assignee)

Comment 10

8 years ago
Created attachment 390989 [details] [diff] [review]
Patch with reftest
Attachment #390482 - Attachment is obsolete: true
Attachment #390989 - Flags: review?(bzbarsky)
Attachment #390989 - Flags: review?(bzbarsky) → review+
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.
(Assignee)

Comment 12

8 years ago
Created attachment 391071 [details] [diff] [review]
Patch with tests and nits

Check in please.
Attachment #390989 - Attachment is obsolete: true
Attachment #391071 - Flags: review+
(Assignee)

Comment 13

8 years ago
Created attachment 391101 [details] [diff] [review]
Patch with tests
Attachment #391071 - Attachment is obsolete: true
Pushed http://hg.mozilla.org/mozilla-central/rev/a8d700960e41
Assignee: hsivonen → dzbarsky
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.