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

Content bug: BatteryManager includes "Additional methods in Mozilla chrome codebase" from EventTarget #3316

Closed
foolip opened this issue Mar 20, 2021 · 4 comments · Fixed by #3388
Assignees
Labels
Content:WebAPI Web API docs needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.

Comments

@foolip
Copy link
Contributor

foolip commented Mar 20, 2021

What page(s) did you find the problem on?

https://developer.mozilla.org/en-US/docs/Web/API/BatteryManager

Specific page section or heading?

https://developer.mozilla.org/en-US/docs/Web/API/BatteryManager#methods

What is the problem?

The source using an include mechanism to inline https://developer.mozilla.org/en-US/docs/Web/API/EventTarget#methods, which includes this section:

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget#additional_methods_in_mozilla_chrome_codebase

What did you expect to see?

Only things which are relevant for BatteryManager. There may be some use to including this information on the EventTarget page, but not on BatteryManager.

@foolip foolip added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Mar 20, 2021
@chrisdavidmills chrisdavidmills added the Content:WebAPI Web API docs label Mar 20, 2021
@hamishwillee hamishwillee self-assigned this Mar 22, 2021
@hamishwillee
Copy link
Collaborator

hamishwillee commented Mar 22, 2021

@wbamberg This is a good test case for discussing page inclusion since the inclusion results in a bug which needs to be fixed.

Would this be covered by the fact that EventTarget is a mixin (ie the discussion here). If so we would document the methods as though they were part of BatteryManager.

@Elchi3 do you have a view on this?

@wbamberg
Copy link
Collaborator

There are a couple of things going on here.

  • BatteryManager inherits from EventTarget. Usually Web API pages handle this by mentioning that fact and linking to the parent, like in, say, https://developer.mozilla.org/en-US/docs/Web/API/HTMLVideoElement#methods. But in this case for no good reason we've decided to handle it by transcluding the pages. We should delete this use of {{page}} and do the normal thing here.

  • EventTarget lists these Mozilla-chrome-specific events. I think we could just delete this section.

In some structured-content nirvana we could automatically slurp properties and methods from a parent into a child.

@Elchi3
Copy link
Member

Elchi3 commented Mar 22, 2021

I agree with @wbamberg here. Mentioning the parent, EventTarget, removing the {{page}} macro, and removing Mozilla-isms is the correct thing here.

Also, this is a case of inheritance and not mixins. See #1006 for a discussion around documentation conventions for inheritance.

@hamishwillee
Copy link
Collaborator

Great. Fixed.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Content:WebAPI Web API docs needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants