Closed Bug 502600 Opened 15 years ago Closed 15 years ago

[HTML5] <!doctype HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN"> should trigger the quirks mode

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
Windows NT
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: jmjjeffery, Assigned: hsivonen)

References

(
URL
)

Details

Attachments

(2 files, 1 obsolete file)

Use == instead of =
788 bytes, patch
mrbkap
: review+
mrbkap
: superreview+
Details | Diff | Splinter Review
mochitest test case
3.56 KB, patch
hsivonen
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
Activate the HTML5 parser in about:config
Load the site in the URL 
Note the display is wrong and appears to be double-triple spaced or more
Turn off the HTML5 parser and reload the site to see the corrrect display of page.

This website also displays wrong with HTML5 enabled:
http://www.sandisk.com/Products/Catalog%281433%29-Players_bundles.aspx
Activewin gets the standards mode with the HTML5 parser but needs the quirks mode.
Sandisk spun off as bug 502804.
Summary: HTML5: Activewin display incorrect with HTML5 enabled → [HTML5] <!doctype HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN"> should trigger the quirks mode
Assignee: nobody → hsivonen
Sigh. A classic missing = bug introduced late in the landing review cycle.
Attachment #387174 - Flags: superreview?(mrbkap)
Attachment #387174 - Flags: review?(mrbkap)
Attachment #387174 - Flags: superreview?(mrbkap)
Attachment #387174 - Flags: superreview+
Attachment #387174 - Flags: review?(mrbkap)
Attachment #387174 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/888b72291dfb

Thanks. Good catch.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Can we add a regression test for this?
Flags: in-testsuite?
Verified fixed using hourly build:

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090708 Minefield/3.6a1pre Firefox/3.0.11 ID:20090708023540

changeset:
http://hg.mozilla.org/mozilla-central/rev/845fd6b1ef23

ActiveWin is now looking normal again. Thanks.
Status: RESOLVED → VERIFIED
Attached patch mochitest test case (obsolete) — — Splinter Review 
I've added this case to test_compatmode.html.  I've also modified that test to run all tests twice:  once without html5.enabled and once with.
Attachment #391713 - Flags: review?(hsivonen)
Comment on attachment 391713 [details] [diff] [review]
mochitest test case

I think it would be easier to follow what the test is doing if running two times were explicit two lines like

doTestWithoutHtml5();
doTestWithHtml5();

instead of the tail of the first run triggering the second run.

Is there a reason for doing it this way?
I've added some comments to the make the test more readable.

I don't think I can structure the test like so:
doTestWithoutHtml5();
doTestWithHtml5();

This is because the test isn't synchronous.  The verification takes place in the iframe's onload handler, and we can't be sure exactly when that will be fired.  If we called the doTest() functions consecutively, there's a chance that the doTestWithHtml5() function would enable the HTML5 parser before the last test in the previous test had completed, or such is my understanding.
Attachment #391713 - Attachment is obsolete: true
Attachment #392744 - Flags: review?(hsivonen)
Attachment #391713 - Flags: review?(hsivonen)
Comment on attachment 392744 [details] [diff] [review]
mochitest test case

OK. I now realize why the the tail of the first run needs to trigger the next run. Please ask someone with a better understanding of the test conventions to sr, though, please.
Attachment #392744 - Flags: review?(hsivonen) → review+
(That is, I don't have proper experience of writing mochitests, so I'm not confident that this is the way it needs to be, even though I now see why it is the way it is.)
Comment on attachment 392744 [details] [diff] [review]
mochitest test case

Bz, can you sr this test per Henri's comments?
Attachment #392744 - Flags: superreview?(bzbarsky)
Attachment #392744 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 392744 [details] [diff] [review]
mochitest test case

Looks ok if we don't do the second pass if html5 parser was enabled during the first pass.  That way we won't be doing two redundant passes once the html5 parser is on by default...

Alternately, file a followup bug on removing the html5 part of things once that parser is on by default?
Depends on: 508867
I opted for option #2 in comment #13; see bug 508867.
Flags: in-testsuite? → in-testsuite+
Blocks: 508867
No longer depends on: 508867
Depends on: 601425
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: