Closed Bug 387691 Opened 17 years ago Closed 17 years ago

Don't ship <link> from the body to the head

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: mp3geek, Assigned: mrbkap)

References

(
URL
)

Details

(Keywords: regression, testcase)

Attachments

(2 files, 2 obsolete files)

testcase
293 bytes, text/html
Details
Fixed fixed fix
7.53 KB, patch
sicking
: review+
sayrer
: review+
jst
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007071020 Minefield/3.0a7pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007071020 Minefield/3.0a7pre

All the main heading fonts are in bright yellow, when viewed in Opera the same fonts are in black. Haven't tested with IE yet.

Reproducible: Always

Steps to Reproduce:
1. Open website
2. View ugly yellow fonts
Actual Results:  
Displayed incorrectly

Expected Results:  
Should show black fonts?
Attached file testcase — 
It happens because <link> get moved into the head, while in older builds, <style> and <link> got moved into the head.
I guess <link> should also be allowed in the <body>, since IE7 allows that too.
Status: UNCONFIRMED → NEW
Component: General → HTML: Parser
Ever confirmed: true
Keywords: regression, testcase
Product: Firefox → Core
QA Contact: general → parser
Version: unspecified → Trunk
Flags: blocking1.9?
Blake, what do you think about this one? 1.9 blocker or not?
Yeah, web compat was P1 last I checked.
Assignee: nobody → mrbkap
Marking blocker per discussion with mrbkap.
Flags: blocking1.9? → blocking1.9+
Attached patch Fix (obsolete) — — Splinter Review 
This patch make the link parent model match the style parent model.
Attachment #272094 - Flags: superreview?(jonas)
Attachment #272094 - Flags: review?(jonas)
Attachment #272094 - Flags: superreview?(jonas)
Attachment #272094 - Flags: superreview+
Attachment #272094 - Flags: review?(jonas)
Attachment #272094 - Flags: review+
Fix checked into trunk.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Summary: The main article fonts are the wrong colour → Don't ship <link> from the body to the head
Whoops, I had to back this out -- <link> was ending up in the body too much. I'll make a new patch post-haste.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
let's add a tree building test too.
Flags: in-testsuite?
I was doing just that when I noticed the bug ;-).
Status: REOPENED → ASSIGNED
Attached patch Fixed fix (obsolete) — — Splinter Review 
This makes two changes:
- It adds kPreferHead (compare to kPreferBody) when ensures that <link> and <style> (and maybe <script> at some point) end up in the head if there hasn't been an explicit <body> yet.

- I changed some of the mochitests to reflect the new behavior. As they stand now, the tests assume that this exact bug exists, which seems bad to me.
Attachment #272094 - Attachment is obsolete: true
Attachment #272253 - Flags: superreview?(jonas)
Attachment #272253 - Flags: review?(sayrer)
Attachment #272253 - Flags: review?(jonas)
Attached patch Fixed fixed fix — — Splinter Review 
I think that this has all of the mochitest stuff set up correctly. I copy/pasted the changed tests into regressions.txt and added the old versions to the exceptions file. I made sure to make small changes to the copy/pasted versions so they don't get ignored as "todo."

I also removed a couple of tabs from the driver script.
Attachment #272253 - Attachment is obsolete: true
Attachment #272261 - Flags: superreview?(jst)
Attachment #272261 - Flags: review?(sayrer)
Attachment #272261 - Flags: review?(jonas)
Attachment #272253 - Flags: superreview?(jonas)
Attachment #272253 - Flags: review?(sayrer)
Attachment #272253 - Flags: review?(jonas)
(In reply to comment #10)
> - It adds kPreferHead (compare to kPreferBody) when ensures that <link> and
> <style> (and maybe <script> at some point) end up in the head if there hasn't
> been an explicit <body> yet.

Just a question: Is html5 scoped style considered at all here?
Even if html5 style's scoped attribute isn't implemented at this point,
perhaps some comments about how it should be handled when implemented 
might be appropriate here.
I'm assuming that kPreferHead would be inappropriate for a scoped style.

Not that I'm a reviewer or anything :)
(In reply to comment #0)
> All the main heading fonts are in bright yellow, when viewed in Opera the same
> fonts are in black. Haven't tested with IE yet.
FWIW, they're showing black in IE7 and Safari 3 Beta (on windows), too.
(In reply to comment #12)
> Just a question: Is html5 scoped style considered at all here?

Not really. The change to not ship <style> out of the body will help with that, but this is really just a compatibility/code simplification change.

> I'm assuming that kPreferHead would be inappropriate for a scoped style.

So, kPreferHead only does something when there has not been any explicit <body> or <frameset>. I haven't looked at what the HTML5 algorithm does in these cases, but I'm more or less trying to restore compatibility with what we used to do.
Attachment #272261 - Flags: review?(sayrer) → review+
Attachment #272261 - Flags: superreview?(jst) → superreview+
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: