Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<li> ordinal value definition is wrong when not directly inside an <ol> #1617

Closed
domenic opened this issue Aug 1, 2016 · 14 comments · Fixed by #2002
Closed

<li> ordinal value definition is wrong when not directly inside an <ol> #1617

domenic opened this issue Aug 1, 2016 · 14 comments · Fixed by #2002

Comments

@domenic
Copy link
Member

domenic commented Aug 1, 2016

(Previously discussed in #1610.)

To solve this, we need to amend the definition of ordinal value to work for all <li>s that render as numbered values. Right now it only works for those directly inside <ol>s (as it references the ol "parent"), but examples like http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4349 and http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4350 shows that browsers render numbers for them even when they are not direct children.

This seems subtle and will probably involve some investigation to find a good algorithm. Browsers are not interoperable, at least for the reverse case, so any algorithm which matches the majority is good.

Some test cases worth thinking about:

  • Are there any elements that will cause the counter to "reset"? (presumably other <ol>s, but maybe also <ul>s?)
  • If different elements or different numbers of elements wrap different <li>s, does that affect the counter?
@domenic domenic added good first issue Ideal for someone new to a WHATWG standard or software project topic: rendering labels Aug 1, 2016
@boogyman
Copy link

boogyman commented Aug 1, 2016

<menu>, <nav>, directly nested <li> (implicit parent tag), and maybe <dir>

//not specifically markup, but it might be good to note the CSS implication
https://www.w3.org/wiki/CSS/Properties/counter-reset

@Manishearth
Copy link

Manishearth commented Aug 1, 2016

https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?q=%2Bfunction%3A%22nsBlockFrame%3A%3AFrameStartsCounterScope%28nsIFrame+%2A%29%22&redirect_type=single#6973

Firefox says ol, ul, dir, menu. nav may be an omission. Edit: looks like nav always contains an ol/ul so I guess it's okay?

@Manishearth
Copy link

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4351

Chrome/Firefox don't renumber inside bare navs. Firefox messes up the numbering due to the nested element, but we already knew that.

@Manishearth
Copy link

It's a bit interesting that value resets the counter in the forward direction, even if the counters are reversed. So

<ol reversed>
  <li>x</li>
  <li value = 2>x</li>
  <li>x</li>
  <li>x</li>
</ol>

gives 4,2,1,0; not 3,2,2,1. This means that user-agents have to execute two traversals -- one to figure out the starting element, and one to actually number them. A single reverse-traversal isn't enough; unlike the single forward traversal that suffices for numbering things in the forward direction.

(As always, my primary reaction to such edge cases is "why would anyone do such a thing" 😝 )

@Manishearth
Copy link

Gecko patch at https://bugzilla.mozilla.org/show_bug.cgi?id=1290813 (need to add reftests and things)

@Manishearth
Copy link

Manishearth commented Aug 1, 2016

Relevant tests https://github.com/w3c/web-platform-tests/blob/master/html/semantics/grouping-content/the-ol-element/reversed-1a.html (etc)

They don't actually have <link rel=match> in them so they get conveniently excluded from the manifest 😄 . I added a fourth test in the Firefox bug which explicitly tests nested divs.

@Manishearth
Copy link

FWIW Gecko opted to merge that, since it brings us closer to other engines (which do more sensible things), and that behavior is unspecced anyway.

Perhaps we should have a "list owner" concept similar to "form owner" for the spec prose? Or simply a "set of child list elements" concept.

"form owner" reminds me -- I wonder how reversed lists work when the user agent receives half a list. (Bit late here, will check tomorrow)

@Manishearth
Copy link

Regarding partial loads, it seems to just renumber when it receives new <li> elements. Seems sane, if amusing.

@domenic
Copy link
Member Author

domenic commented Oct 17, 2016

@Manishearth I noticed that as you pointed out in #1617 (comment) there are a bunch of nice tests for this now. We should really get the spec fixed, and preferably fix #1911 at the same time (/cc @upsuper). Although maybe I'll tackle that first since it seems easier.

Could you help us get started on writing a spec here? Maybe describe the algorithm you implemented in Gecko?

@domenic
Copy link
Member Author

domenic commented Oct 17, 2016

Also, have you looked in to the case of elements that are not being rendered, and how they impact the numbering? Per http://jsbin.com/jekokitonu/edit?html,css,js,output it seems like only the being-rendered <li>s are numbered. fun.

@upsuper
Copy link
Member

upsuper commented Oct 17, 2016

Yes, only rendered <li>s are counted.

@Manishearth
Copy link

Manishearth commented Oct 21, 2016

(sorry about the delay, missed this)

Maybe describe the algorithm you implemented in Gecko?

It's the same algorithm as the one for numbering, except:

  • It doesn't listen to value
  • It doesn't actually number things

So I do one traversal to count what I would number, and a second one to actually number them (this time obeying value resets). This was implemented by just adding a boolean flag to the function for numbering.

Perhaps we can spec it like this?

  • Redefine ordinal value of an li to be the "assigned value for the li in the steps for numbering an ol with the closest parent ol element for which this li element is an associated li element" (with a note that there should be only one such ol, if any)
  • Define numbering an ol to be an algorithm which produces the assigned value for each associated li element where you:
    • do a traversal of associated li elements of the ol in tree order, starting with i equal to
      • 1 if the ol is not reversed and has no start
      • start if the ol has a start attribute
      • the number of associated li elements if the ol is reversed but has no start attribute
    • In each iteration:
      • If the li element has a value attribute, set i to value
      • set the assigned value for the li element to i
      • increment or decrement i based on the reversed attribute of the ol
  • Define associated li element for a given ol as an li element:
    • Which is rendered
    • Which is a descendant of the given ol element
    • Which is not a descendant of any menu, ol, ul, li, dir elements which are descendants of the given ol element

I suggest a note giving the example of the algorithm in Gecko or something would be helpful here to clarify things.

@sendilkumarn
Copy link
Contributor

Just an opinion...

Why can't we have an algorithm similar to normal in case of reverse

  • If reversed,
    • start from last tag
    • if value is present, set the value to the defined value and start reducing, while navigating backwards.
<ol reversed>
   <li> x </li>
   <li value = 5 > x </li>
   <li> x </li>
   <li> x </li>
</ol>

4,5,2,1

@upsuper
Copy link
Member

upsuper commented Oct 28, 2016

That probably makes sense, but that doesn't match browsers' current behavior. And unless you can convince browsers to take the risk of breaking existing content and change their behavior to yours, we are pretty much stuck on the current behavior.

domenic added a commit that referenced this issue Nov 1, 2016
domenic added a commit that referenced this issue Nov 8, 2016
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
@domenic domenic removed the good first issue Ideal for someone new to a WHATWG standard or software project label Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants