Add MDN annotations to spec output

[sideshowbarker]

This change adds annotations in the margin of the spec output next to any items for which there’s a corresponding MDN article somewhere under https://developer.mozilla.org/en-US/docs/Web that has a Specifications section with a link to that item's URL.

The change gives wattsi a new required command-line argument for specifying the filename of a JSON file containing a mapping of spec ID-attribute values to MDN article pathnames.

Addresses whatwg/html-build#180

[sideshowbarker]

https://arcane-cove-57093.herokuapp.com/multipage/ has sample output from this.

It currently adds 761 annotations

[domenic]

Overall code seems nice and straightforward; I guess the complexity is all in assembling the JSON file :). What remains is bikeshedding on the markup and CSS.

Currently this produces markup like the following:

<span class="mdn">
  <span class=""><b onclick="toggleStatus(this)">MDN</b> <a href="https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API">Web_Workers_API</a></span>
  <span class=""><b onclick="toggleStatus(this)">MDN</b> <a href="https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Using_web_workers">Web_Workers_API/Using_web_workers</a></span>
</span>

I would prefer something more like

<div class="mdn" onclick="toggleStatus(this)">
 <strong>MDN</strong>
 
 <ul>
  <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API">Web_Workers_API</a></li>
  <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Using_web_workers">Web_Workers_API/Using_web_workers</a></li>
 </ul>
</div>

(or maybe aside instead of div, although I notice we use p for caniuse for some reason...)

WDYT?

[domenic]

Oh, also: if it's possible to gather the data, using the article title instead of URL piece might be good?

[sideshowbarker]

So the difficultly I ran into with using div instead of span is that it makes the output invalid. The reason is, many of the elements these annotations end up getting appended to are p elements

So when that gets parsed the div causes the parser to close that p element, and thus when I run the output through the HTML checker, it spits out tons of No “p” element in scope but a “p” end tag seen.

Would have the same problem with using ul inside the annotations.

A possible way around that would be to insert the annotation for those cases not into the parent node of the element being annotated but instead before the next sibling of that parent node.

But when I tried that, the necessary CSS got uglier (need to use negative margins) — but worse than that, in some cases it resulted in the annotation getting appended to the end of body.

If I spent more time looking at those instances, I reckon I might be figure out how to special-case for them. But in the end, it just seemed a lot simpler and cleaner to use span.

[sideshowbarker]

Outside of the validation/parsing/styling issue with using a div/ul/li structure instead: In the vast majority of cases (maybe even literally 99%), the annotation contains only a single MDN link. And in some sections, there are lot of the annotations right after each other, though not in the same paragraph (some paragraphs are only a single line).

So given all that, it seems like putting the MDN on a separate line in the annotation, above the link title could end up causing a lot of the annotation boxes to overlap with each. So just having the MDN part and link title on the same line seemed a better fit as far as styling.

And that allows the entire annotation to be collapsed down to just the MDN prefix part in far right the margin — by clicking on the MDN prefix part itself instead of the link-title hypertext — in cases where the annotation is overlapping the spec text.

[sideshowbarker]

Oh, also: if it's possible to gather the data, using the article title instead of URL piece might be good?

Yes, it’s possible. It just means I’ll need to have the data-file build take the source of each linked-to article and run it through an HTML parser to extract the title. I was anyway already planning to do that in order to extract the class=seoSummary parts from the articles that have those — so that we can take that and put it into the title text for the hyperlink (so it’ll be displayed on hover).

[domenic]

Can we put the div in the sample place that we currently put caniuse boxes, where it's valid? That is directly inside the <body>, but near the relevant node I guess. (So, crawl up until you find a direct child of the body, then put it previous to that node?)

For example right now a lot of these spans are inside IDL code blocks, but they're definitely not code, so that's not great. In particular if you drag-select the IDL you end up with MDN stuff:

I take the point about there only being a single article in most cases. No <ul> is probably fine then, although it wouldn't hurt I think.

[sideshowbarker]

Oh, also: if it's possible to gather the data, using the article title instead of URL piece might be good?

Yes, it’s possible. It just means I’ll need to have the data-file build take the source of each linked-to article and run it through an HTML parser to extract the title. I was anyway already planning to do that in order to extract the class=seoSummary parts from the articles that have those — so that we can take that and put it into the title text for the hyperlink (so it’ll be displayed on hover).

448a352 updates that wattsi code do that; that is, the link hypertext in the annos is now the MDN article title, and when you hover over the link, the article summary will be shown.

[sideshowbarker]

Can we put the div in the sample place that we currently put caniuse boxes, where it's valid? That is directly inside the <body>, but near the relevant node I guess. (So, crawl up until you find a direct child of the body, then put it previous to that node?)

Yeah, I can try that for the IDL blocks at least.

For example right now a lot of these spans are inside IDL code blocks, but they're definitely not code, so that's not great. In particular if you drag-select the IDL you end up with MDN stuff

Yeah, I agree that’s bad

[sideshowbarker]

https://arcane-cove-57093.herokuapp.com/multipage/ has the output at this point (with the change made to use the article title and show the article summary on hover)

[domenic]

Ooof, now I am remembering how bad the MDN titles are... "SharedWorkerGlobalScope.close()", but that doesn't exist, it's SharedWorkerGlobalScope.prototype.close(). Not sure what to do here. I wish MDN would stop being misleading.

[zcorpan]

Have you reported an issue about that?

[sideshowbarker]

Can we put the div in the sample place that we currently put caniuse boxes, where it's valid? That is directly inside the <body>, but near the relevant node I guess.

Yeah, I can try that for the IDL blocks at least.

I changed it for the links within p elements as well. The annos for elements that have a p ancestors are inserted after the end of p element — because putting it there rather than before the p minimizes the cases where annos with long titles overlap the spec text). The annos for pre elements are inserted before the pre element — because that’s what makes more sense for those.

(So, crawl up until you find a direct child of the body, then put it previous to that node?)

It doesn’t exactly crawl all the way up to find a direct child of the body — because that doesn’t produce a good result in some cases. Specifically, there are some elements with referenced IDs that are children of dt and td elements or that are themselves th elements. In those cases to get the annos aligned at point of use, the annos are appended to the referenced elements themselves.

https://arcane-cove-57093.herokuapp.com/multipage/ has the current output

[sideshowbarker]

Ooof, now I am remembering how bad the MDN titles are... "SharedWorkerGlobalScope.close()", but that doesn't exist, it's SharedWorkerGlobalScope.prototype.close(). Not sure what to do here. I wish MDN would stop being misleading.

Yeah I agree all those should be fixed. I’ll ask about it on #mdn

[sideshowbarker]

Ooof, now I am remembering how bad the MDN titles are... "SharedWorkerGlobalScope.close()", but that doesn't exist, it's SharedWorkerGlobalScope.prototype.close(). Not sure what to do here. I wish MDN would stop being misleading.

Yeah I agree all those should be fixed. I’ll ask about it on #mdn

Discussed on #mdn and still not sure what’d be the best way to proceed further, but it seems like starting a thread on https://discourse.mozilla.org/c/mdn could be a good way

[domenic]

Posted https://discourse.mozilla.org/t/incorrect-titles-for-method-property-articles/31641

[domenic]

LGTM. Let me move the CSS.

[sideshowbarker]

So is there anything else we’re waiting on before merging this?

[sideshowbarker]

So is there anything else we’re waiting on before merging this?

[sideshowbarker]

So is there anything else we’re waiting on before merging this?

[domenic]

I'm a little scared of merging two order-dependent PRs with GitHub in its current fragile state, and how well any subsequent Travis runs and deploys will go. So if we could wait until https://status.github.com/ calms down a bit that'd be ideal. But yeah nothing blocking on the code front.