Invalid TYPE attribute makes UL look like OL

VERIFIED FIXED in Firefox 46

Status

()

defect
--
minor
VERIFIED FIXED
15 years ago
2 years ago

People

(Reporter: jruderman, Assigned: xidorn)

Tracking

({dev-doc-complete, regression, testcase})

Trunk
mozilla46
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 -
wanted1.9.2 -

Firefox Tracking Flags

(firefox46 fixed)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

15 years ago
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!
(Reporter)

Comment 1

15 years ago
Posted file testcase
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...

Updated

15 years ago
OS: Windows XP → All
Hardware: PC → All

Comment 3

13 years ago
*** Bug 318746 has been marked as a duplicate of this bug. ***

Comment 4

13 years ago
*** Bug 336571 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Keywords: regression

Updated

11 years ago
Flags: wanted1.9.2?
(Reporter)

Comment 5

11 years ago
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-

Updated

9 years ago
Duplicate of this bug: 547661

Comment 7

4 years ago
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
(Assignee)

Comment 8

4 years ago
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
(Assignee)

Comment 9

4 years ago
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)

Comment 10

4 years ago
(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

Comment 11

4 years ago
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.
(Assignee)

Comment 14

4 years ago
Bug 241719 - Do not override list-style-type if the list element has invalid type value.
Attachment #8676054 - Flags: review?(dbaron)

Comment 15

4 years ago
dev-doc-info
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)

Updated

4 years ago
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+
(Assignee)

Comment 17

4 years ago
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.
(Assignee)

Comment 18

3 years ago
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)
(Assignee)

Comment 21

3 years ago
(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+
(Assignee)

Comment 23

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b505d3974c64be842cbaeff3de76a60d319382e7
Bug 241719 part 1 - Do not override list-style-type if the list element has invalid type value. r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/46430a7e2af9d10956066c456bcc58fcdebddbb4
Bug 241719 part 2 - Add test for this bug. r=dbaron

Comment 24

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b505d3974c64
https://hg.mozilla.org/mozilla-central/rev/46430a7e2af9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46

Comment 26

3 years ago
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

Comment 27

2 years ago
Posted file testcase.html
Bug not fixed in Firefox 55.0.2
Please see this test case.

Comment 28

2 years ago
(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!

Updated

2 years ago
Blocks: 1392682
You need to log in before you can comment on or make changes to this bug.