254445 - 32372 fallout

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[fantasai]

Figure out what Gecko is doing to HTTP headers, particularly Default-Style,
where it is storing them, and where they're being accessed from.
(Document this while you're at it.)

Note: we're failing
  http://www.hixie.ch/tests/evil/css/import/extra/alternate.http.html ?


Remove GetPreferredSheet from nsCSSLoader and use a GetHeader call unless this
turns out to cause problems.

Comment 1

[Peter Van der Beken [:peterv]]

nsDocument::SetHeaderData stores the value of the headers and passes the
preferred stylesheet to the CSS loader, so the CSS loader's value is always the
same as stored in nsDocument's mHeaderData (that's why I questioned the need for
nsCSSLoader::GetPreferredSheet).

Regular HTTP headers are handled in nsContentSink::ProcessHTTPHeaders (which
calls nsContentSink::ProcessHeaderData).
<META http-equiv="Default-Style"> is handled in nsContentSink::ProcessMETATag
(which calls nsContentSink::ProcessHeaderData).

nsContentSink::ProcessHeaderData calls nsDocument::SetHeaderData.

Stylesheets loads from HTTP headers and <META httpEquiv="link"> go through
nsContentSink::ProcessStyleLink (which calls
nsDocument::SetHeaderData(nsHTMLAtoms::headerDefaultStyle, ...) if
nsDocument::GetHeaderData(nsHTMLAtoms::headerDefaultStyle, ...) is empty).
Stylesheets loads from PIs, LINK elements, STYLE elements go through
nsStyleLinkElement::UpdateStyleSheet (which calls
nsDocument::SetHeaderData(nsHTMLAtoms::headerDefaultStyle, ...) if
nsDocument::GetHeaderData(nsHTMLAtoms::headerDefaultStyle, ...) is empty).

Hope that helps.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Peterv is right.  The reason I used GetHeaderData is that it's the right thing
to use...

> Note: we're failing
>  http://www.hixie.ch/tests/evil/css/import/extra/alternate.http.html ?

That's a bug in the testcase, as far as I can tell.  See bug 184524 and note the
HTTP headers in this case:

Link: <preferred1.css>; rel="stylesheet alternate"; type="text/css";
      title="Preferred Set"
Default-Style: "Preferred Set"

Comment 3

[fantasai]

Ok, but are we storing the actual unmodified headers anywhere? I just find it a
bit strange that a function that is supposed to return HTTP headers returns the
effective value instead. It's like returning the effective charset as determined
by auto-detect. Do we do that, too? Is this a "header information, most of which
happen to be set by HTTP headers" function or an "HTTP headers that were set on
this document" function? If it's the former, it's not well-documented in the
nsDocument header file. If it's the latter, then setting Default-Style like that
is wrong.

Thoroughly confused,
fantasai

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Ok, but are we storing the actual unmodified headers anywhere?

On the channel.

> I just find it a bit strange that a function that is supposed to return HTTP
> headers 

The documentation for this function says "this may also get set from other sources"

> It's like returning the effective charset as determined by auto-detect. Do we
> do that, too?

GetHeaderData doesn't know about charsets.  The nsIDocument functions that do
most certainly know about autodetect results...

Really, GetHeaderData would be better called GetMetaData or something; we have
been talking about using it for random metadata for a while.

Comment 5

[fantasai]

"this may also get set from other sources"
I take this to mean from HTTP-equiv and any other places that are explicitly
designed to "set HTTP headers", not from HTML4 algorithms for deriving
preferredStylesheetSet from link tags!

To fix this bug, I would
  - rename GetHeaderData to GetMetaData or GetMetaHeaders or something and update
    the comments documenting it
  - remove GetPreferredSheet from nsCSSLoader and use GetMetaData instead
Does that sound right?

Comment 6

[Boris Zbarsky [:bzbarsky]]

That sounds fine to me, but I'd like to hear jst's opinion...

Comment 7

[fantasai]

Changing summary & purpose of bug report. Here's yet another comment:

------- Additional Comment #135 From Darin Fisher (IBM) 2004-08-27 18:39 PDT
[reply] -------

It looks like nsIMarkupDocumentViewer was changed without modifying the UUID of
the interface.  Sure, it's a private interface, but you never know if some wacky
extension or embedder may be using this interface for some random reason. 
Better to always modify interface UUIDs when modifying interfaces.  Otherwise,
you give those who need to use this interface no hope of doing so across browser
versions.


*note to self* I also need to fix a XUL mistake and some whitespace.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #7)
> It looks like nsIMarkupDocumentViewer was changed without modifying the UUID of
> the interface.

uuid is now changed.

Comment 9

[fantasai]

jst, can you reply to comment 5, please?

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.