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

Remove mixins that have been removed from MDN content #3131

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Mar 4, 2021

Mixin work is so thankless. Really.
I forgot to remove mixin pages from the infamous InterfaceData and GroupData macros.

This is what is happening: Mixin pages are made redirects to some sensible real interface pages like Element.
Great for readers because they'll land somewhere, but these lovely macros still make calls to get the sub pages and so they can be rendered twice (once from Element, and also from e.g. NonDocumentTypeChildNode which is now a redirect to Element.)

I'll add this to my mixin checklist, so it doesn't happen again.

Closes #3119

@Elchi3
Copy link
Member Author

Elchi3 commented Mar 4, 2021

Also, sidebars do really weird things and we should make them stop

@wbamberg
Copy link
Collaborator

wbamberg commented Mar 4, 2021

Oh, so it's this bit:

var implementPages = await page.subpagesExpand('/en-US/docs/Web/API/' + impl[i]);
collect(implementPages);

?

@Elchi3
Copy link
Member Author

Elchi3 commented Mar 4, 2021

Yeah exactly.

@wbamberg
Copy link
Collaborator

wbamberg commented Mar 9, 2021

Tiny PR, huge comment. sorry.

I understand some of this but not all of it.

What it looks like this is doing

It looks like this is doing the following things:

Changes in GroupData

  • remove the NonDocumentTypeChildNode interface from the "DOM" group
  • remove the HTMLHyperlinkElementUtils interface from the "URL API" group

Removing interfaces from groups ought to stop that interface from showing up in the sidebar for group-level pages.

Changes in InterfaceData

  • remove NonDocumentTypeChildNode from CharacterData.impl
  • remove NonDocumentTypeChildNode from Element.impl
  • remove HTMLHyperlinkElementUtils and URLUtilsSearchParams from HTMLAnchorElement.impl
  • remove HTMLHyperlinkElementUtils and URLUtilsSearchParams from HTMLAreaElement.impl

Removing interfaces from X.impl properties ought to stop members of that interface from being listed in interface X, and this specifically is what causes the double-counting in the interface-level sidebars.

Things I understand

Currently, at https://developer.mozilla.org/en-US/docs/Web/API/Document_Object_Model, NonDocumentTypeChildNode shows up in the sidebar under "Interfaces". Because NonDocumentTypeChildNode is a mixin that you've removed and redirected to Element, the link just redirects to https://developer.mozilla.org/en-US/docs/Web/API/Element. With the copy of GroupData.json from the PR, the NonDocumentTypeChildNode interface link no longer appears. Yay.

Also, listing NonDocumentTypeChildNode under Element.impl is what makes some items show up twice in the sidebar on https://developer.mozilla.org/en-US/docs/Web/API/Element. With the copy of InterfaceData.json from the PR, that's fixed. Yay.

Things I don't understand

Removing NonDocumentTypeChildNode from CharacterData.impl

In this PR, NonDocumentTypeChildNode is also removed from CharacterData.impl.

But currently on MDN, the sidebar for https://developer.mozilla.org/en-US/docs/Web/API/CharacterData is not double-counting members from NonDocumentTypeChildNode:


Screen Shot 2021-03-09 at 2 32 56 PM


(although links like ariaAtomic are going to the new page under /Element)

In the sidebar built from this PR, things like ariaAtomic don't appear at all:


Screen Shot 2021-03-09 at 2 34 06 PM


So what's happening here?

Does CharacterData actually implement ariaAtomic, etc (in which case, I guess, we should have copies of it under the CharacterData page, as we do for Element? Or does it not (which looks likely actually) meaning that this CharacterData.impl entry was never correct?

Removing HTMLHyperlinkElementUtils from the "URL API" group

HTMLHyperlinkElementUtils is removed from the "URL API" group. Does this imply that it's a mixin and should be deleted? But I still see https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils, although its members now redirect to pages under HTMLAnchorElement (e.g. https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils/hash). Similar to the situation with Element and NonDocumentTypeChildNode, I would have expected to see duplicate items in the sidebar for https://developer.mozilla.org/en-US/docs/Web/API/HTMLAnchorElement. But I don't, and I don't understand why not.

Removing URLUtilsSearchParams is removed from HTML*.impl

URLUtilsSearchParams is removed from HTML*.impl. But URLUtilsSearchParams doesn't exist at all in GroupData, although URLSearchParams and URLUtilsReadOnly both do. And https://developer.mozilla.org/en-US/docs/Web/API/URLUtilsSearchParams is a 404. Was this an error in the old data, referencing a nonexistent interface?

@Elchi3
Copy link
Member Author

Elchi3 commented Mar 10, 2021

Sorry to not have explained this in more details. Basically, there are two messy areas colliding here: mixins and sidebars. It is no fun.

Removing NonDocumentTypeChildNode from CharacterData.impl

Or does it not (which looks likely actually) meaning that this CharacterData.impl entry was never correct?

It was correct. However, with the redirect NonDocumentTypeChildNode -> Element in place,
the NonDocumentTypeChildNode entry in CharacterData.impl it behaves like NonDocumentTypeChildNode is Element, which is wrong. As a consequence ariaAtomic and friends get added in a sidebar where they have never been added before.

Removing HTMLHyperlinkElementUtils from the "URL API" group

HTMLHyperlinkElementUtils is removed from the "URL API" group. Does this imply that it's a mixin and should be deleted?

Yes, it is a mixin and I deleted the page. However, MDN has trouble deleting things. See #2224

I would have expected to see duplicate items in the sidebar for https://developer.mozilla.org/en-US/docs/Web/API/HTMLAnchorElement. But I don't, and I don't understand why not.

I think that's because HTMLHyperlinkElementUtils isn't a redirect. It didn't become anything like Element, so the sidebar macro doesn't pull anything from a redirect.

Removing URLUtilsSearchParams is removed from HTML*.impl

URLUtilsSearchParams is removed from HTML*.impl. But URLUtilsSearchParams doesn't exist at all in GroupData, although URLSearchParams and URLUtilsReadOnly both do. And https://developer.mozilla.org/en-US/docs/Web/API/URLUtilsSearchParams is a 404. Was this an error in the old data, referencing a nonexistent interface?

I removed this because it is old data. If I remember correctly it is the old name for HTMLHyperlinkElementUtils. Mixins get renamed all the time. Another reason to avoid documenting them directly.

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 this pull request may close these issues.

Sidebar is broken for the API/Element page
3 participants