1201600 - MDN tooltip needs to work when "Summary" heading is missing

Status: RESOLVED WONTFIX

(DevTools :: Inspector, defect, P3)

Description

[Will Bamberg [:wbamberg]]

The MDN tooltip widget in the Inspector works by scraping the HTML for CSS reference pages. Among other things, it looks for the "Summary" H2 heading to find the text to use for the summary[1] in the tooltip.

The MDN writing team has decided to remove this heading, since it's redundant. This then breaks that part of the tooltip.

This isn't catastrophic: the tooltip still works, and still displays the syntax and link to the page, which are both more important than the summary. But the widget should be updated so it can handle the new format (perhaps falling back to including the first paragraph in the document, if the heading can't be found?).

Long term, this kind of thing just highlights how much MDN needs an API to provide access to content, if we are to go on doing things like this.


[1] https://dxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/MdnDocsWidget.js?offset=0#441

Comment 1

[Sebastian Zartner [:sebo]]

(In reply to Will Bamberg [:wbamberg] from comment #0)
> The MDN tooltip widget in the Inspector works by scraping the HTML for CSS
> reference pages. Among other things, it looks for the "Summary" H2 heading
> to find the text to use for the summary[1] in the tooltip.

The summary (actually only the first sentence of it) could be retrieved by appending '$json'[1] or '?raw&summary'[2] to the URL.
Though because that requires an additional server request, this may not be the best solution.

> This isn't catastrophic: the tooltip still works, and still displays the
> syntax and link to the page, which are both more important than the summary.
> But the widget should be updated so it can handle the new format (perhaps
> falling back to including the first paragraph in the document, if the
> heading can't be found?).

Yes, falling back to the first paragraph would be a good solution until there is a proper API.

> Long term, this kind of thing just highlights how much MDN needs an API to
> provide access to content, if we are to go on doing things like this.

I guess what is needed here is ability to specify which sections should be retrieved, right? Is there already a bug for that? If not, I can create one.

Sebastian

[1] https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-border-bottom-colors$json
[2] https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-border-bottom-colors?raw&summary

Comment 2

[Will Bamberg [:wbamberg]]

(In reply to Sebastian Zartner [:sebo] from comment #1)
> (In reply to Will Bamberg [:wbamberg] from comment #0)
> > The MDN tooltip widget in the Inspector works by scraping the HTML for CSS
> > reference pages. Among other things, it looks for the "Summary" H2 heading
> > to find the text to use for the summary[1] in the tooltip.
> 
> The summary (actually only the first sentence of it) could be retrieved by
> appending '$json'[1] or '?raw&summary'[2] to the URL.
> Though because that requires an additional server request, this may not be
> the best solution.

Right, I didn't use the API specifically because I didn't want to make 2 separate network requests.

> > This isn't catastrophic: the tooltip still works, and still displays the
> > syntax and link to the page, which are both more important than the summary.
> > But the widget should be updated so it can handle the new format (perhaps
> > falling back to including the first paragraph in the document, if the
> > heading can't be found?).
> 
> Yes, falling back to the first paragraph would be a good solution until
> there is a proper API.
> 
> > Long term, this kind of thing just highlights how much MDN needs an API to
> > provide access to content, if we are to go on doing things like this.
> 
> I guess what is needed here is ability to specify which sections should be
> retrieved, right? Is there already a bug for that? If not, I can create one.

I don't know of a bug for it. Yes, I guess, for this feature we'd want to be able to ask for "summary" and "syntax-by-example" (and maybe compatibility in future?).

Comment 3

[Sebastian Zartner [:sebo]]

(In reply to Will Bamberg [:wbamberg] from comment #2)
> (In reply to Sebastian Zartner [:sebo] from comment #1)
> > > Long term, this kind of thing just highlights how much MDN needs an API to
> > > provide access to content, if we are to go on doing things like this.
> > 
> > I guess what is needed here is ability to specify which sections should be
> > retrieved, right? Is there already a bug for that? If not, I can create one.
> 
> I don't know of a bug for it. Yes, I guess, for this feature we'd want to be
> able to ask for "summary" and "syntax-by-example" (and maybe compatibility
> in future?).

With bug 1232391 I created request for a new API now that targets the current use case.
Regardless of that the different APIs may be streamlined in the future, so people have a central method to access all the data related to articles.

Sebastian

Comment 4

[Julian Descottes [:jdescottes]]

Inspector bug triage. Filter on CLIMBING SHOES.

Comment 5

[Sebastian Zartner [:sebo]]

I'd really like to get rid of the 'Summary' headings. And I think there may be an alternative to bug 1232391 to find the summary.

On the fetched document you first need to search for document.querySelector(".seoSummary"). If that exists, use its contents as summary, otherwise search for document.getElementsByTagName("p"), loop over the returned HTMLCollection. If the element only contains macros, go to the next one, otherwise use its contents as summary.

I.e. something like this:

function getSummary(mdnDocument) {
  let summaryElement = mdnDocument.querySelector(".seoSummary");
  if (!summaryElement) {
    let i = 0;
    while(i < paragraphs.length - 1 && /^\s*\{\{/.test(paragraphs[i].textContent)) {
      i++;
    }
    summaryElement = paragraphs[i];
  }
  return summaryElement.innerHTML;
}

Will, do you think that would be ok as interim solution until bug 1232391 is fixed?

Sebastian

Comment 6

[Will Bamberg [:wbamberg]]

Yes, I think this sounds fine. Except we use textContent not innerHTML.

Comment 7

[Eric Shepherd [:sheppy]]

Yeah, using .seoSummary is how MDN's macros fetch summaries for tooltips, fwiw.

Comment 8

[Sebastian Zartner [:sebo]]

To repeat what I've told Will in Hawaii, I've created a patch for this, which worked well in browser though let the automated tests fail for some reason.

I just didn't find the time yet to look into it again and post the patch here for review. I'll try to do that the next days.

Sebastian

Comment 9

[Will Bamberg [:wbamberg]]

I think we should remove this feature from the devtools.

It gets very little use, according to the telemetry data (fewer than 4000 uses in all of 2016), and it stops us making some much-needed improvements to the CSS pages. For example, it will break if we move the "Syntax example" section to the top of the page, which we want to do, and it would break again if we ever want to have interactive examples in the pages.

I'd love us to have a good way to integrate CSS docs into the Inspector, but this isn't it.

Patrick, what do you think?

Comment 10

[Sebastian Zartner [:sebo]]

(In reply to Will Bamberg [:wbamberg] from comment #9)
> I think we should remove this feature from the devtools.
> 
> It gets very little use, according to the telemetry data (fewer than 4000
> uses in all of 2016)

That's a really poor number! Though I assume the main reason for this is the UI.

> and it stops us making some much-needed improvements
> to the CSS pages. For example, it will break if we move the "Syntax example"
> section to the top of the page, which we want to do, and it would break
> again if we ever want to have interactive examples in the pages.
> 
> I'd love us to have a good way to integrate CSS docs into the Inspector, but
> this isn't it.

For me it would be ok to remove it for now in order to be able to do long-standing MDN changes and reintroduce the feature with a better UI later.
I really hope we can get a better integration with the DevTools this year as discussed at the All Hands!

Sebastian

Comment 11

[Patrick Brosset <:pbro>]

Wow, 4000 in a year! That's really low indeed. These could all be us testing it too!
I tweeted about the feature not long ago https://twitter.com/FirefoxDevTools/status/807361582371979264 to remind people about its existence, and got some decent response.

A better UI would make a ton of difference, but also a smarter UI, we need a way to know what people really care about and provide it to them in an easy when they need it. 

For instance, in bug 1306054, we're trying to show when properties are "useless" in the rule-view. We could show an MDN info right there, like we do in the console or netmonitor. Same when property values are invalid, we could link to MDN so people can check which values are valid.

So, I do agree that we should remove the current feature.
Bryan: what do you think?

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

Yeah, that's pretty low.  I think we could safely turn it off for now and review how to implement it again another time.

Comment 13

[Sebastian Zartner [:sebo]]

Ok, I've created bug 1352801 to remove/disable the tooltip and mark this as WONTFIX, then.

Sebastian