Jesse Ruderman Reporter
	Description • 20 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!

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 2 • 20 years ago

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...

	Erik Fabert
	Comment 3 • 18 years ago

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

	Erik Fabert
	Comment 4 • 18 years ago

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

	Jesse Ruderman Reporter
	Comment 5 • 16 years ago

jj says this worked correctly in Firefox 1.0 and not in Firefox 1.5.

	j.j.
	Comment 7 • 9 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

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 8 • 9 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

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 9 • 9 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?

	j.j.
	Comment 10 • 9 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

	j.j.
	Comment 11 • 9 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

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 12 • 9 years ago

(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.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 13 • 9 years ago

That said, it's not clear to me why this happens at all, given the code in HTMLSharedListElement::ParseAttribute and HTMLSharedListElement::MapAttributesIntoRule.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 14 • 9 years ago

Bug 241719 - Do not override list-style-type if the list element has invalid type value.

	Jean-Yves Perrier [:teoli]
	Comment 15 • 9 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.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 16 • 9 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

https://reviewboard.mozilla.org/r/22559/#review21639

Indeed.  Thanks for fixing.  Maybe add a test?  (Should be easy enough as a testharness.js test.)

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 17 • 9 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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 18 • 8 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/

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 19 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/29181/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29181/

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 20 • 8 years ago

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?

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 21 • 8 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

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 22 • 8 years ago

Comment on attachment 8702783 [details]
MozReview Request: Bug 241719 part 2 - Add test for this bug.

https://reviewboard.mozilla.org/r/29181/#review25989

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 23 • 8 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

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b505d3974c64
https://hg.mozilla.org/mozilla-central/rev/46430a7e2af9

	Jean-Yves Perrier [:teoli]
	Comment 25 • 8 years ago

Added a sentence in https://developer.mozilla.org/en-US/Firefox/Releases/46#HTML

	j.j.
	Comment 26 • 8 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

	Seb
	Comment 27 • 7 years ago

Bug not fixed in Firefox 55.0.2
Please see this test case.

	j.j.
	Comment 28 • 7 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!