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)
Core
DOM: HTML Parser
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)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Updated•26 years ago
|
Target Milestone: M7
Updated•25 years ago
|
Target Milestone: M7 → M8
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. ;-)
This is now fixed with Necko. main NGLayout code can ask for any HTTP header it
wants using the channel.
Updated•25 years ago
|
Whiteboard: will attempt to verify when release builds available
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Updated•25 years ago
|
Resolution: FIXED → ---
Comment 4•25 years ago
|
||
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.
Comment 6•25 years ago
|
||
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).
Updated•25 years ago
|
Comment 7•25 years ago
|
||
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.
Updated•25 years ago
|
Assignee: vidur → peterl
Comment 8•25 years ago
|
||
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).
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 9•25 years ago
|
||
Moving non-beta 1 items to M15
Reporter | ||
Comment 10•25 years ago
|
||
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.)
Comment 11•25 years ago
|
||
I believe refresh headers were/are supposed to be handled down in network or
parser code...
Comment 12•25 years ago
|
||
Reassigning peterl's bugs to myself.
Comment 13•25 years ago
|
||
Accepting peterl's bugs that have a Target Milestone
Comment 14•25 years ago
|
||
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.)
Comment 15•25 years ago
|
||
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).
Updated•25 years ago
|
Whiteboard: will attempt to verify when release builds available
Comment 16•25 years ago
|
||
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.
Reporter | ||
Comment 17•25 years ago
|
||
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?
Comment 18•25 years ago
|
||
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.
Comment 19•25 years ago
|
||
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...
Comment 20•25 years ago
|
||
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.
Reporter | ||
Comment 21•25 years ago
|
||
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".
Comment 22•25 years ago
|
||
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.
Comment 23•25 years ago
|
||
Bulk move of all Networking-Core (to be deleted component) bugs to new
Networking component.
Comment 24•25 years ago
|
||
Pushing my M15 bugs to M16
Comment 25•25 years ago
|
||
spam, changing qa contact from paulmac to tever@netscape.com on networking/RDF
bugs
QA Contact: paulmac → tever
Comment 26•25 years ago
|
||
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
Comment 27•25 years ago
|
||
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
Reporter | ||
Comment 29•24 years ago
|
||
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
Keywords: correctness,
testcase
QA Contact: janc → py8ieh=bugzilla
Whiteboard: hit during nsbeta2 standards compliance testing
Assignee | ||
Comment 30•24 years ago
|
||
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
Reporter | ||
Comment 31•24 years ago
|
||
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.
Reporter | ||
Comment 32•24 years ago
|
||
Marking ns6.01 so that we fix this ASAP after the 6.0 release.
Keywords: ns6.01
Reporter | ||
Updated•24 years ago
|
Summary: HTTP headers are not passed on to main NGLayout code → HTTP headers are not passed on to main NGLayout code [IMPORT]
Updated•24 years ago
|
OS: Windows 98 → All
Hardware: PC → All
Reporter | ||
Comment 33•24 years ago
|
||
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.
Assignee | ||
Comment 34•24 years ago
|
||
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)
Comment 35•24 years ago
|
||
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);
Assignee | ||
Comment 37•24 years ago
|
||
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.
Comment 38•24 years ago
|
||
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.
Comment 39•24 years ago
|
||
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)
Assignee | ||
Comment 40•24 years ago
|
||
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.
Comment 42•24 years ago
|
||
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.
Assignee | ||
Comment 43•24 years ago
|
||
Fix is in. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•