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

[[FEAT]] Added window globals and DOM interfaces to browser #3522

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JuanFML
Copy link

@JuanFML JuanFML commented Dec 17, 2020

[[FEAT]] Added window globals and DOM interfaces to browser

Added DOM interfaces and Window globals into the exports.browser object in vars.js. In order to cover more globals .

Fixes #3495

@coveralls
Copy link

coveralls commented Dec 17, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 9ef48d1 on JuanFML:add-globals-browser into 0c240e6 on jshint:master.

@JuanFML
Copy link
Author

JuanFML commented Dec 17, 2020

Can somebody help me with the errors? I don't understand where the problem is.

@jugglinmike
Copy link
Member

Thanks for the patch! The build errors were due to an unrelated problem with
one of this project's transitive dependencies. I just fixed
that
, though, so things should
work as intended if you push another commit to this branch.

As for this patch, it introduces a good deal more bindings than what has been
requested in gh-3495. Some of them, like toolbar, are non-standard and
deprecated. Other bindings, like WindowGlobalScope are not defined by any
browser. JSHint ought to issue warnings when either of those kinds of bindings
are used, so they shouldn't be listed in the vars.js file.

In order to avoid confusion like this, we've taken to annotating new additions
with code comments which explain where the bindings have been standardized. If
you could do the same here, it would be more clear why each of the new bindings
is necessary.

@jugglinmike
Copy link
Member

@JuanFML It's been a few months since we heard from you. Are you still interested in persuing this patch?

@JuanFML
Copy link
Author

JuanFML commented Feb 13, 2021

Hello @jugglinmike I am sorry I haven’t been able to give it some time, if anyone wants to finish it or should we close the PR?

@Ashesh3
Copy link
Contributor

Ashesh3 commented Aug 19, 2021

@jugglinmike @JuanFML I would like to take this up and finish it. Before i proceed though, i would want to confirm how exactly you want the new additions to be annotated?

AudioTrack, // https://html.spec.whatwg.org/multipage/media.html#audiotrack
DOMParser, // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#domparser
.....

Is it like above or

//Taken from https://html.spec.whatwg.org/multipage/indices.html#all-interfaces
AudioTrack,
DOMParser
....

Or like this? Or any other format. Please let me know and i would try to finish this PR.

@jugglinmike
Copy link
Member

The former approach seems good to me!

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

Successfully merging this pull request may close these issues.

Setting browser:true in .jshintrc does not cover some globals
4 participants