667518 - Don't trim white-space from <link> href attribute value

Status: VERIFIED FIXED

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

Description

[Mats Palmgren (inactive)]

Bug 649134 added trimming of white-space from <link> @href.

AFAICT, the "Editor's Response" in 
http://www.w3.org/Bugs/Public/show_bug.cgi?id=12508#c2
says we should not do that, and explicitly mentions that we should
load a non-empty white-space-only @href.

Comment 1

[Boris Zbarsky [:bzbarsky]]

OK.  Want to fix?

Comment 2

[Mats Palmgren (inactive)]

Attachment: fix

Remove the whitespace Trim() from nsHTMLLinkElement::GetStyleSheetURL().

Not sure what to do about nsContentSink::ProcessLinkHeader() though.
Stripping WS there dates back to 1999 at least, so it seems too risky
to remove that at this point, although rfc 3986 does seem to suggest
it should NOT be stripped when quoted.  What do you think?

"Using <> angle brackets around each URI is especially recommended as
a delimiting style for a reference that contains embedded whitespace."
http://tools.ietf.org/html/rfc3986#appendix-C

http://tools.ietf.org/html/rfc5988#section-5

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to comment #2)
> Not sure what to do about nsContentSink::ProcessLinkHeader() though.
> Stripping WS there dates back to 1999 at least, so it seems too risky
> to remove that at this point, although rfc 3986 does seem to suggest
> it should NOT be stripped when quoted.  What do you think?

I would guess it wouldn't be too risky to remove Link header support altogether (since the header isn't supported by all browsers), so it's probably safe to change space handling. Do we even know of any real-world uses of the Link header outside the sites of standards experts who work for browser vendors?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> contains embedded whitespace

I would read that as whitespace in the middle of the URI, not on the edges.  So I think trimming is fine.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 543101 [details] [diff] [review]
fix

r=me

Comment 6

[Mats Palmgren (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ed9bdae57981

Comment 7

[Marco Bonardo [:mak]]

backed out from inbound because the added test is permaorange on Android R2, please fix it or mark it as fail-if android on repush.

Comment 8

[Mats Palmgren (inactive)]

Re-pushed with the test disabled for Android:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f0d8fa54fecb

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/f0d8fa54fecb

Comment 10

[Ioana (away)]

Verified fixed on:
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0

Verified by loading beta reftests in the browser: 649134-1, 649134-2, 649134-2-ref.
649134-2 is displayed the same way as 649134-2-ref (moz - blue, ie - block surrounded by a red line), which means the href white-spaces are not trimmed. If the white-spaces were trimmed, 649134-2 would have been displayed the same way as 649134-1 (style specifications would be ignored).

This fix was verified the same way on the latest aurora and central builds.