Closed Bug 3248 Opened 26 years ago Closed 24 years ago

HTTP headers are not passed on to main NGLayout code [IMPORT]

Categories

(Core :: DOM: HTML Parser, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9

People

(Reporter: ian, Assigned: harishd)

References

()

Details

(Keywords: arch, highrisk, testcase, Whiteboard: [fix in hand ] hit during nsbeta2 standards compliance testing (py8ieh:need small testcases for lots of headers))

Attachments

(2 files)

HTTP headers are not passed on to the main NGLayout code. For example, any LINK HTTP headers should be parsed and passed on to the document to emulate <LINK> elements. This page does not refresh: http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/refresh1.http.html (See also bug #563, redirects) The first two tests here are not green: http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/cascade/acidlinkcascade.html Test 65 does not go green: http://www.bath.ac.uk/%7Epy8ieh/internet/importtest/extra/index.html
Status: NEW → ASSIGNED
Target Milestone: M7
Target Milestone: M7 → M8
Blocks: 7232
Pushed past necko landing...
Changing all Networking Library/Browser bugs to Networking-Core component for Browser. Occasionally, Bugzilla will burp and cause Verified bugs to reopen when I do this in a bulk change. If this happens, I will fix. ;-)
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
This is now fixed with Necko. main NGLayout code can ask for any HTTP header it wants using the channel.
Whiteboard: will attempt to verify when release builds available
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
These tests do not pass on my windows system. The first two tests at acidlinkcascade.html are not green, and test 65 does not pass. I do not know enough about these tests to provide further info right now.
Assignee: gagan → valeski
Status: REOPENED → NEW
Jud: who (docloader/webshell) should get this bug now?
The first url doesn't contain any refresh headers (therefore it's not refreshing). The other two urls deal w/ LINK meta tags. Gagan, what does "LINK" do in http? I'm wondering if this is a case in which we need to have our back channel into necko from layout (ugh).
Assignee: valeski → vidur
No longer blocks: 7232
Target Milestone: M9 → M10
re-assigning to vidur (he's my only layout contact ;) and moving to M10 as I don't see this bumping other M9 dev work. I'm also removing 7232's dependency on this bug, as this is now a layout issue.
Assignee: vidur → peterl
This falls into Peter Linss's domain. The HTTP response header is now available off the nsIChannel passed into the HTMLContentSink and should be queried for link headers (section 19.6.2.4 of http://www.cis.ohio-state.edu/htbin/rfc/rfc2068.html).
Status: NEW → ASSIGNED
Moving non-beta 1 items to M15
Hmm, http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/refresh1.http.html does have a refresh header! This is what the server sends me: HTTP/1.1 200 OK Date: Tue, 12 Oct 1999 20:03:53 GMT Server: Apache/1.3.9 (Unix) Refresh: 3;url=refresh2.http.html <<<< NOTICE THIS LINE! <<< Last-Modified: Wed, 16 Jun 1999 00:37:41 GMT ETag: "8061-877-3766f1d5" Accept-Ranges: bytes Content-Length: 2167 Connection: close Content-Type: text/html It is not refreshing. (It works in IE5.)
I believe refresh headers were/are supposed to be handled down in network or parser code...
Reassigning peterl's bugs to myself.
Accepting peterl's bugs that have a Target Milestone
I believe this is a layout bug. You should be able to access the Refresh header by QI'ing the nsIChannel object to an nsIHTTPChannel and calling GetHeader (or something like that). Gagan can give you the details. After you get the Refresh header, you have to add the magic that forces the refresh after the specified timeout. Necko doesn't do that because it needs to involve the docloader/webshell/current document, etc. (I believe.)
The magic that causes the refresh is already there. <meta http-equiv="Refresh: ..." or whatever it is already works fine (see http://www.psych.upenn.edu/~baron/david/satellite/ ). The only thing that needs to be done is using information from the HTTP header in the same place as the info from the META. (The same thing applies to Link elements in HTTP headers and probably a few other things.) It sounds like this should be pretty easy to fix, and it's blocking a few features of Bugzilla, so I hope you can do it soon :-) There is one question though... for headers that can only exist once (like Refresh, and unlike Link (I think)), does the META take precedence or does the HTTP header win? HTML 4.0 is annoyingly silent on this issue (or at least I couldn't find anything it says). This should be tested with the Content-Type header too (charset parameter).
Whiteboard: will attempt to verify when release builds available
although the meta refresh code does a content refresh and re-load, a different path should be taken to Refresh the page for the HTTP header. The meta stuff is designed to handle *only* HTTP-EQUIV headers.
Well, that is probably not a good idea. The code that handles HTTP headers and the code that handles HTTP equivalent headers found within the markup should be one and the same. After all, almost every HTTP header can be also included in HTTP-equiv tags. Why duplicate the code?
I share your concern. HTTP-EQUIV is a very, very, very *evil* hack. It's only relevent in client's that can handle changing HTTP headers (the definition of that is hard to narrow down too) *after* the actual HTTP transaction has taken place. All the browser clients I can think of handle this as a hack which bites. The problem is that you have content dictating a network level change. It flat out doesn't make sense. Thus, the code is seperate. BTW: per some specification (don't recall off the top of my head which) *any* HTTP header can be represented as an HTTP-EQUIV header. This complicates the situation in a large way.
Ian - one of the other things about refresh headers is that they can be used with images and things that are part of a document, rather than the main document. Probably the existing code only handles the case of the object being the document. However, it would probably be good if the HTTP-EQUIV code could let the network code do the work, when possible. Or something...
The LINK HTTP header, defined in section 19.6.1.2 of RFC 2068 (the original HTTP 1.1 spec), appears in RFC 2616 (June 1999 HTTP 1.1 spec, obsoletes 2068) only in the following context in section 19.6.3 "Changes from RFC 2068": >The Alternates, Content-Version, Derived-From, Link, URI, Public and >Content-Base header fields were defined in previous versions of this >specification, but not commonly implemented. See RFC 2068 [33]. This looks like abandonment, which makes no good sense: a consistent set of links for every document at a server could be very useful. The HTML 4 spec has this to say (in the subsection cited below): >Preferred style sheets specified with META or HTTP headers have precedence >over those specified with the LINK element. This is unchanged in the HTML 4.01 spec, presumably edited after drafts of the new HTTP 1.1 spec were available. Unless or until that statement is removed from the HTML spec, continuing to use the content of any "Link" HTTP header makes sense to me. On the topic of precedence of HTTP-EQUIV versus natural HTTP headers: Quoting from http://www.w3.org/TR/REC-html40/present/styles.html#h-14.3.2 : >If two or more META declarations or HTTP headers specify the preferred style >sheet, the last one takes precedence. HTTP headers are considered to occur >earlier than the document HEAD for this purpose. Generalized, this would mean that any <META HTTP-EQUIV="" CONTENT=""> would over-ride the CONTENT of the header (if any) for which it an EQUIValent. A corollary: the final state of the HTTP header environment for an HTML document is not knowable until the <HEAD> is closed explicitly or can no longer be open. The example of <META http-equiv="Default-Style" content="compact"> taking precedence over preferred stylesheets defined with LINK in the same subsection of the HTML 4 spec works correctly: it applies the stylesheet titled "compact" even if another preferred stylesheet is LINKed first (tested with 1999-12-05-16-M12 on WinNT). The cleanest implementation would probably be to use the same parser for META HTTP-EQUIV equivalent-to-headers as for actual HTTP headers, and change any and all headers once only, when the <HEAD> is closed, propagating any resultant changes at that time as if the headers with substituted content had always been that way, even if that has implications that invalidate everything done for the current document so far.
Dan Connolly and I are working together to write a new ID for the "Link" HTTP header, although due to other work this project has been delayed. It was removed from the HTTP ID for the exact reasons given: they were not commonly implemented. As I understand it, for a feature to make it into an ID, it generally needs two client implementations, two proxy implementations, and two server implementations (if relevant each time). BTW, IIRC Mozilla also supports the "Content-Base" HTTP header (when used in a META tag, anyway). I agree that the ideal implementation is one where we use one parser for both real and meta HTTP headers, but I disagree that the </head> tag should have any relevance. If there is ever any sort of equivalent mechanism for XML docs, it is likely to be allowed anywhere (as a PI), so we should simple act on the headers when we get them, not delay. This is, IIRC, what we do now for both "link" and "content-base".
The relevance of </HEAD> *for HTML documents only* is that the last preferred stylesheet specified with META HTTP-EQUIV takes precedence, and until the HEAD is closed, there is no way to say that there isn't another. Given that in HTML the META element may not occur in the BODY, it does not look like there is anything to be gained by speculatively loading each preferred stylesheet specified by META HTTP-EQUIV in turn when normally the HEAD will be explicity or implicitly closed in less elapsed time than it would take to receive the first packet of the first stylesheet, even if this were done in parallel (non-pipelined). Whether done in parallel or not, if there is more than one preferred stylesheet linked in by META HTTP-EQUIV at least one would not be applied; beginning to load it would cause a perceptible and wasteful delay before content appears on a slow link. Point noted for XML - does this apply to XHTML also? I would agree that any stylesheet linked in *after* the close of the HEAD should be applied immediately, but this is not incompatible with waiting for the </HEAD> for any linked in *before* that.
Bulk move of all Networking-Core (to be deleted component) bugs to new Networking component.
Pushing my M15 bugs to M16
spam, changing qa contact from paulmac to tever@netscape.com on networking/RDF bugs
QA Contact: paulmac → tever
Despite what Vidur wrote on 1999-08-16, I'm really enclined to side with Peter when we wrote on 1999-10-12 "I believe refresh headers were/are supposed to be handled down in network or parser code...". Pushing to M18. It sounds like the Necko folks have done their part. Reassigned to Parser.
Assignee: pierre → rickg
Status: ASSIGNED → NEW
Component: Networking → Parser
QA Contact: tever → janc
Target Milestone: M16 → M18
Harish: I don't know whether this is a legit issue, or just folks passing the buck. Please take a look (I'm passing the buck, too!) : )
Assignee: rickg → harishd
putting on nsbeta3 radar.
Status: NEW → ASSIGNED
Keywords: nsbeta3
jan: Since this is (was) mainly an issue with the CSS Import Test, I'm gonna grab QA. Feel free to take it back if you want it...
Blocks: html4.01
QA Contact: janc → py8ieh=bugzilla
Whiteboard: hit during nsbeta2 standards compliance testing
spoke with Ian and he's okay with FUTURING this bug. This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.
Target Milestone: M18 → Future
Removing the nsbeta3 nomination (which was added by Harish) because he was agreed to Futuring this bug. Marking bug 'arch'. *As I understand it* this is an archicture problem. We need to make sure we get to it very early on after RTM.
Keywords: nsbeta3arch
Priority: P3 → P2
Whiteboard: hit during nsbeta2 standards compliance testing → hit during nsbeta2 standards compliance testing (py8ieh:need small testcases for lots of headers)
Marking ns6.01 so that we fix this ASAP after the 6.0 release.
Keywords: ns6.01
Summary: HTTP headers are not passed on to main NGLayout code → HTTP headers are not passed on to main NGLayout code [IMPORT]
OS: Windows 98 → All
Hardware: PC → All
Blocks: 34135
Marking "highrisk arch mozilla0.9" to indicate that this is a fundamental change to the architecture, but we've been putting it off for a while now, but that we really want this in for the next release.
Keywords: ns601highrisk, mozilla0.9
Keywords: nsbeta1
Target Milestone: Future → mozilla0.9
Attached patch proposed patch v1.1 (deleted) — Splinter Review
Whiteboard: hit during nsbeta2 standards compliance testing (py8ieh:need small testcases for lots of headers) → [fix in hand ] hit during nsbeta2 standards compliance testing (py8ieh:need small testcases for lots of headers)
couple questions. 1. is your tabbing->space setting in line w/ the rest of the file? There seems to be some allignment missmatch. I can't tell if your fixing whitespace, or injecting inconsistencies. 2. you're getting the "default load group" to acquire a channel. are you sure that gives you the channel corresponding w/ the current document? Feels like you might get the wrong channel.
A few comments. The indentation in the files seems to be weird - are there tabs? Could you fix that? I think you could avoid one variable & QI here, when you get the request just QI directly to nsIHTTPChannel: + nsCOMPtr<nsIRequest> request; + loadgroup->GetDefaultLoadRequest(getter_AddRefs(request)); + if(request) { - nsCOMPtr<nsIChannel> channel(do_QueryInterface(request)); - if(channel) { ! nsCOMPtr<nsIHTTPChannel> httpchannel(do_QueryInterface(request)); + if (httpchannel) { You could simply say "while(*name) {": + while(*name!=0) { I would like to see here a check if we got key, and if not, return NS_ERROR_OUT_OF_MEMORY: + key=NS_NewAtom(*name++); nsXPIDLCString has a member function get() that returns const char* so you don't need the cast. Also, did you try this without the temporary variable "value", just passing tmp to ProcessHeaderData()? We have lots of automation happening with strings: + nsAutoString value; + value.AssignWithConversion(NS_STATIC_CAST(const char*, tmp)); + ProcessHeaderData(key,value);
Jud: 1) The alignment gets wacky, sometimes, when I use -uw flags. 2) rpotts suggested using default load group to get the appropriate document channel. However, passing down the channel to the sink, when the sink gets constructed in the document, would probably clear the ambiguity. I can make the change, if you insist.
your call on the ambiguity. if you did make it explicit, you'd be shielded from any semantic changes, or bugs, in the default load group implementation. if rick suggested it, I'm sure it's gold though. your call.
Isn't DefaultLoadRequest the first request on a loadgroup? Are you sure that is what you want? To me it seems that if you have an image with one of these headers that you care about, you might miss that header if that image sits on a document. Just double-check that with rick... otherwise looks ok to me (with heikki and jud's suggestions)
r=heikki, although I suspect if we get out of memory situation we should probably bail out immediately and not try to do anything else as that is also likely to fail. Your call.
sr=vidur with a few recommendations: 1) I'd like to see nsCOMPtrs used for referencing the nsIAtoms that are created. 2) You can avoid an extra copy of the header value in ProcessHTTPHeaders by wrapping the returned value in a nsLiteralString and making ProcessHeaderData accept a nsAReadableString& for the value. 3) I didn't see the diff for the prototype for NS_NewHTMLContentSink (in nsHTMLParts.h, I believe). Similar code probably needs to sit in nsXMLContentSink, but that's another problem and one that could be dealt with when you consolidate the content sinks.
Fix is in. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → FIXED
excellent :-D
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: