Closed Bug 241719 Opened 21 years ago Closed 9 years ago

Invalid TYPE attribute makes UL look like OL

Categories

(Core :: Layout, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: jruderman, Assigned: xidorn)

References

()

Details

(Keywords: dev-doc-complete, regression, testcase)

Attachments

(4 files)

The author of this page meant to write <ul type="disc"> (unnecessary, since that's the default) but wrote <ul type="disk"> instead. As a result, Mozilla displays the list as a numbered list!
Attached file testcase (deleted) —
Yeah, this is because type is handled in frame code instead of content/style code. I hope to fix that when converting lists to be done using counters and ::marker, but it could be fixed pretty easily if that doesn't happen soon...
Depends on: 3247
OS: Windows XP → All
Hardware: PC → All
Depends on: 288704
*** Bug 318746 has been marked as a duplicate of this bug. ***
*** Bug 336571 has been marked as a duplicate of this bug. ***
Keywords: regression
Flags: wanted1.9.2?
jj says this worked correctly in Firefox 1.0 and not in Firefox 1.5.
Flags: wanted1.9.2?
Flags: wanted1.9.2-
Flags: blocking1.9.2-
Just waiting 11 years made this bug INVALID due to CSS spec changes, right? https://drafts.csswg.org/css-counter-styles-3/#extending-css2
Depends on: 966166
No, I think this is unrelated to the CSS Counter Styles spec at all. That spec says nothing about the "type" attribute on UL/OL. I believe it is the scope of the HTML specs. I don't see there is "type" attribute on <ul> in the HTML5 spec [1], while in HTML4.01, "type" is a deprecated attribute for <ul> and <ol> [2]. Gecko also implemented "type" attribute for <li>, but I don't see this in either HTML5 or HTML4. [1] https://html.spec.whatwg.org/multipage/semantics.html#the-ul-element [2] http://www.w3.org/TR/html4/struct/lists.html#h-10.3.1
No longer depends on: 966166
I believe fixing this nowadays should be fairly simple: we could just remove attribute mapping from the frame code and use the UA style sheet instead. dbaron, do you agree with this change?
Flags: needinfo?(dbaron)
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #8) > Gecko also implemented "type" attribute for <li>, but I don't see this in > either HTML5 or HTML4. > > [1] https://html.spec.whatwg.org/multipage/semantics.html#the-ul-element > [2] http://www.w3.org/TR/html4/struct/lists.html#h-10.3.1 "type on li" is an "obsolete feature" in HTML https://html.spec.whatwg.org/multipage/obsolete.html#non-conforming-features relevant is the renering section for presentational hints https://html.spec.whatwg.org/multipage/rendering.html#lists I'm wondering: Is it clear from both specs (HTML and CSS), what to do here - if not, needs the HTML spec a clarification and is it worth to do
hmm, my previous comment was partly nonsense. Nothing has changed on the fact that type=unknown should be ignored in HTML and this bug is valid
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #9) > I believe fixing this nowadays should be fairly simple: we could just remove > attribute mapping from the frame code and use the UA style sheet instead. > > dbaron, do you agree with this change? No, since the UA style sheet rule will be considerably slower.
Flags: needinfo?(dbaron)
That said, it's not clear to me why this happens at all, given the code in HTMLSharedListElement::ParseAttribute and HTMLSharedListElement::MapAttributesIntoRule.
Bug 241719 - Do not override list-style-type if the list element has invalid type value.
Attachment #8676054 - Flags: review?(dbaron)
Adding d-d-n as the bug is pretty old and (I think) worth a note in the relevant Fx for devs page.
Keywords: dev-doc-needed
Assignee: nobody → quanxunzhen
Comment on attachment 8676054 [details] MozReview Request: Bug 241719 part 1 - Do not override list-style-type if the list element has invalid type value. r=dbaron https://reviewboard.mozilla.org/r/22559/#review21639 Indeed. Thanks for fixing. Maybe add a test? (Should be easy enough as a testharness.js test.)
Attachment #8676054 - Flags: review?(dbaron) → review+
It seems the manifest updater of web-platform-tests doesn't work properly on Windows. I've filed bug 1222850 for that. I'll add a test and submit for review after that bug gets fixed.
Comment on attachment 8676054 [details] MozReview Request: Bug 241719 part 1 - Do not override list-style-type if the list element has invalid type value. r=dbaron Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22559/diff/1-2/
Attachment #8676054 - Attachment description: MozReview Request: Bug 241719 - Do not override list-style-type if the list element has invalid type value. → MozReview Request: Bug 241719 part 1 - Do not override list-style-type if the list element has invalid type value. r=dbaron
Why didn't the existing tests in the same directory catch the bug? A number of the existing tests in this directory are currently marked as failing... shouldn't the patch have made them pass, and changing the expectations be sufficient? And how did you check that it's ok to land tests here? Is this known to be bidirectionally mirrored?
Flags: needinfo?(quanxunzhen)
(In reply to David Baron [:dbaron] ⌚️UTC-5 from comment #20) > Why didn't the existing tests in the same directory catch the bug? A number > of the existing tests in this directory are currently marked as failing... > shouldn't the patch have made them pass, and changing the expectations be > sufficient? No. With the first patch, all tests there keep getting expected results. The reason is that, the "unsupported" tests there only check whether "ul" and "ol" accept values of each other. This behavior isn't changed in this bug. What this bug does is, values which are accepted by neither "ul" nor "ol" should be treated as invalid and ignored, instead of acting like decimal. > And how did you check that it's ok to land tests here? Is this known to be > bidirectionally mirrored? It seems to me people just keep adding new tests directly there: https://hg.mozilla.org/mozilla-central/rev/5ebc59281c25 https://hg.mozilla.org/mozilla-central/rev/95b404586f1d
Flags: needinfo?(quanxunzhen)
Attachment #8702783 - Flags: review?(dbaron) → review+
Comment on attachment 8702783 [details] MozReview Request: Bug 241719 part 2 - Add test for this bug. https://reviewboard.mozilla.org/r/29181/#review25989
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Mozilla/5.0 (Windows NT 5.1; rv:46.0) Gecko/20100101 Firefox/46.0 The reported URL from 2004 is alive - VERIFIED fixed in Nightly, broken before
Status: RESOLVED → VERIFIED
Attached file testcase.html (deleted) —
Bug not fixed in Firefox 55.0.2 Please see this test case.
(In reply to Seb from comment #27) > Created attachment 8899887 [details] > testcase.html > > Bug not fixed in Firefox 55.0.2 > Please see this test case. confirming, this bug is fixed for <ul type=invalid>, not for <ul style=list-style:invalid>. Will file anew bug, thanks!
Blocks: 1392682
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: