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

fix-headers incorrectly messes with non-section headers #419

Closed
halindrome opened this issue Apr 1, 2015 · 17 comments · Fixed by #421
Closed

fix-headers incorrectly messes with non-section headers #419

halindrome opened this issue Apr 1, 2015 · 17 comments · Fixed by #421

Comments

@halindrome
Copy link
Contributor

If a document has a bunch of h* elements within a section, where these elements are not ALL the same (e.g., the section heading is an h3 and all of the headings within the section are h4), this module will incorrectly match on the subheadings and tweak them. Basically, the CSS selector is naive. Thanks @ianbjacobs for pointing this out.

@darobin
Copy link
Member

darobin commented Apr 1, 2015

The issue here is that you shouldn't use hN that aren't section titles. I guess ReSpec should actually flag that as an error.

@halindrome
Copy link
Contributor Author

I disagree strongly. There are lots of reasons to use headers.

halindrome added a commit that referenced this issue Apr 1, 2015
Only the first heading within a section should be
renumbered to match depth.

Also, fix-headers.js and structure.js were doing the same
thing to the H elements.  I removed the duplicate code from
structure.js since it was redundant.

This fixes Issue #419.
@marcoscaceres
Copy link
Member

I disagree strongly. There are lots of reasons to use headers.

Can you provide some examples?

@halindrome
Copy link
Contributor Author

Sure. But more importantly, ReSpec shouldn't be proscriptive. If an author wants to use <h5> to indicate that something is a header, why would we prevent them?

In today's crisis, the Web Payments interest group has a number of use cases. The major headings that go into the ToC are of course hN elements and ReSpec correctly numbers them. Within these use cases there are sub items that are NOT "sections". However, the editors have chosen to identify the various sub-cases with hN elements for the headers.

This resulted in styling challenges because ReSpec was munging the headers randomly.

See https://dvcs.w3.org/hg/webpayments/raw-file/default/latest/use-cases/index.html#cryptocurrency-payment-bitcoin-ripple

@darobin
Copy link
Member

darobin commented Apr 1, 2015

What is the header a header of if not of a section? Using headers without sections only relies on the outline algorithm, which really, really isn't great. ReSpec is prescriptive whenever there's a good practice.

@halindrome
Copy link
Contributor Author

I am confused. What outline algorithm?

And regardless of what authors should be doing, the behavior ReSpec currently exhibits is clearly incorrect. It shouldn't mess with some headers and not with others. That doesn't make any sense.

@darobin
Copy link
Member

darobin commented Apr 1, 2015

The algorithm that outlines an HTML document.

It's only processing those headers differently because they are misplaced. If you put your abstract at the end it will also do strange things!

@halindrome
Copy link
Contributor Author

Not sure how that is relevant. But again, regardless, I think that respec should either touch ALL of the headers within a section or only the first one, but not some random number that is based upon what hN elements the author happened to use in the source. The fix I have provided has respec adjust the level of the first item, which is clearly what the code was trying to do originally.

@msporny
Copy link
Member

msporny commented Apr 1, 2015

@darobin @marcoscaceres Can you suggest a different way of separating content within a section where you don't want those "separators" to show up in the ToC?

For example, how would you do this layout w/o multiple <hN>s?

https://dvcs.w3.org/hg/webpayments/raw-file/default/latest/use-cases/index.html#cryptocurrency-payment-bitcoin-ripple

What ReSpec does right now violates the "Principle of Least Astonishment" http://en.wikipedia.org/wiki/Principle_of_least_astonishment . When I saw the output, I spent a non-trivial chunk of time trying to debug the CSS before I figured out it was ReSpec mucking w/ the header markup. I put the markup back to something that worked, and then @ianbjacobs hit the same issue trying to do exactly what I was trying to do. We were specifying an <h3> and ReSpec was replacing it with an <h4>. I don't understand why ReSpec needs to treat the first two <hN> values differently than each one thereafter.

ReSpec also violates the HTML5 spec's assertion that you can use multiple <hN>s inside of a <body/section> (doing so results in a semantically equivalent document):
http://www.w3.org/TR/html5/sections.html#the-h1,-h2,-h3,-h4,-h5,-and-h6-elements

I agree w/ Shane on this one. ReSpec should either:

  1. Provide a feature that allows you to opt-out of placing a section into the ToC and numbering it, or
  2. Don't do wacky stuff w/ <hN>s

@gkellogg
Copy link
Member

gkellogg commented Apr 1, 2015

Looking at structure.js, there are a few hacks you might try. I do think that having a "notoc" class you could apply to a header, or a section might be a good idea, though:

  • Include your toc-less sections in a span with @class="introductory"
  • Use a header >= h7

If the notoc class was acceptable, it could be a one line change to structure.js:

isIntro = $sec.hasClass("introductory") || $sec.hasClass("notoc")

and, perhaps a bit lower:

isIntro ||= h.hasClass("notoc")

@marcoscaceres
Copy link
Member

notoc is what we used to use for anolis, I believe. It worked ok.

@msporny
Copy link
Member

msporny commented Apr 1, 2015

I'd be happy w/ the "notoc" solution. Thanks to @gkellogg for figuring out what it would take to implement it, and @halindrome for raising the issue and proposing an alternate solution. :)

Thinking a bit more on this... if the "notoc" solution is implemented, I think ReSpec is still violating the rule of least astonishment as well as what the HTML5 spec says. I don't care that deeply about this case if there is a clear workaround, but it seems like I shouldn't have to use the "notoc" workaround for this to "just work". It feels like ReSpec is forcing me to markup my sections in a particular way, even though the semantics are the same between:

<section>
  <h2>Header A</h2>
</section>
<section>
  <h2>Header B</h2>
</section>

and

<h2>Header A</h2>
<h2>Header B</h2>

Don't want to create any more work than necessary, so if it's technically difficult to do this in ReSpec, then I'm fine w/ using "notoc" and moving on.

@marcoscaceres
Copy link
Member

@msporny, I used to think the same (in fact, I refused to use ReSpec for a long time precisely because of the issue you outline), but I've come around to using sections over the last few years. Yes, it's really annoying having to use sections to recreate the sectioning algorithm (specially when you are starting a doc), but it's not that big a deal.

@gkellogg
Copy link
Member

gkellogg commented Apr 1, 2015

I think sections are important for semantic reasons; headers only, it can be difficult to know where the text following the header ends, particularly if there are multiple headers with different levels inside the same doc or section. To me, it makes sense that the section bounds the content, and the header describes it.

@marcoscaceres
Copy link
Member

@gkellogg, yeah, opinions vary there... and ReSpec agrees with you, hence its usage of section (Anolis didn't agree, for instance, so it established implied sections based on hN levels automatically ... not sure what Bikeshed does).

Anyway, I would not object to a PR that added no notoc support.

@msporny
Copy link
Member

msporny commented Apr 2, 2015

+1 to implementing "notoc" and moving on.

@darobin
Copy link
Member

darobin commented Apr 2, 2015

The semantics are the same, the processing isn't. Processing documents with implicit sections is a pain in the arse when you need to do anything that actually involves sections. It also means that people get things wrong by using the wrong header level (particularly when moving content around).

ReSpec isn't HTML. It is a deliberately restricted (and extended) vernacular. Mixing sectioned headers and unsectioned headers is like mixing tabs and spaces in a document. ReSpec is actually pretty opinionated in a number of ways, you probably don't notice most of the time because you follow similar practices yourself :)

Violating least astonishment is relative to expectations. The bug here is that it soldiered on without warning you that it thought the input was wrong. I've opened #422 for that purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants