Last Comment Bug 484200 - src attribute on <style> makes styles not apply
: src attribute on <style> makes styles not apply
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: ---
Assigned To: David Zbarsky (:dzbarsky)
:
Mentors:
http://software.hixie.ch/utilities/js...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-19 08:28 PDT by Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-09-26)
Modified: 2009-07-28 09:10 PDT (History)
5 users (show)
jst: wanted‑next+
jst: blocking1.9.2-
jst: wanted1.9.2+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.04 KB, patch)
2009-07-24 09:29 PDT, David Zbarsky (:dzbarsky)
bzbarsky: review-
bzbarsky: superreview-
Details | Diff | Splinter Review
Patch with reftest (10.67 KB, patch)
2009-07-27 19:45 PDT, David Zbarsky (:dzbarsky)
bzbarsky: review+
Details | Diff | Splinter Review
Patch with tests and nits (10.67 KB, patch)
2009-07-28 06:45 PDT, David Zbarsky (:dzbarsky)
bzbarsky: review+
Details | Diff | Splinter Review
Patch with tests (10.67 KB, patch)
2009-07-28 09:05 PDT, David Zbarsky (:dzbarsky)
no flags Details | Diff | Splinter Review

Description Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-09-26) 2009-03-19 08:28:50 PDT
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) (Not reading bugmail or doing reviews until 2016-09-26) 2009-03-19 08:35:51 PDT
See also http://software.hixie.ch/utilities/js/live-dom-viewer/saved/36
Comment 2 Boris Zbarsky [:bz] (TPAC) 2009-05-04 21:37:55 PDT
Does anyone support <style src="..."> other than us?  I'd be quite happy to rip it out.
Comment 3 Boris Zbarsky [:bz] (TPAC) 2009-05-04 21:39:54 PDT
For that matter, we don't seem to do a very good job of supporting it either.
Comment 4 Boris Zbarsky [:bz] (TPAC) 2009-05-04 21:41:56 PDT
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) (Not reading bugmail or doing reviews until 2016-09-26) 2009-05-05 18:34:00 PDT
Yeah, I wanted to zap the code.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2009-07-23 15:33:36 PDT
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) 2009-07-24 09:29:49 PDT
Created attachment 390482 [details] [diff] [review]
patch
Comment 8 Boris Zbarsky [:bz] (TPAC) 2009-07-27 11:22:24 PDT
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 [:bz] (TPAC) 2009-07-27 11:23:09 PDT
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) 2009-07-27 19:45:14 PDT
Created attachment 390989 [details] [diff] [review]
Patch with reftest
Comment 11 Boris Zbarsky [:bz] (TPAC) 2009-07-28 06:18:07 PDT
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) 2009-07-28 06:45:07 PDT
Created attachment 391071 [details] [diff] [review]
Patch with tests and nits

Check in please.
Comment 13 David Zbarsky (:dzbarsky) 2009-07-28 09:05:59 PDT
Created attachment 391101 [details] [diff] [review]
Patch with tests
Comment 14 Boris Zbarsky [:bz] (TPAC) 2009-07-28 09:10:52 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/a8d700960e41

Note You need to log in before you can comment on or make changes to this bug.