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

Align Element.ScrollIntoView with the latest draft spec #927

Closed
wolfbeast opened this Issue Jan 8, 2019 · 1 comment

Comments

Projects
None yet
1 participant
@wolfbeast
Copy link
Member

wolfbeast commented Jan 8, 2019

We currently have a spec alignment issue with Element.ScrollIntoView in that the IDL doesn't provide the expected enumeration ScrollLogicalPosition values (which breaks some versions of React), and our function definitions are wrong.

See:
https://drafts.csswg.org/cssom-view/#dictdef-scrollintoviewoptions
https://drafts.csswg.org/cssom-view/#dom-element-scrollintoview

The IDL is supposed to be this:

enum ScrollLogicalPosition { "start" , "center", "end", "nearest" };
dictionary ScrollIntoViewOptions : ScrollOptions {
  ScrollLogicalPosition block = "center";
  ScrollLogicalPosition inline = "center";
};

So if an empty object/dict is passed in as a parameter, it will default to using center/center for the dict's entries. When nothing is passed in, it's supposed to instead default to start/end.

Our IDL says:

  void scrollIntoView(boolean top);
  void scrollIntoView(optional ScrollIntoViewOptions options);

but the spec's says:

  void scrollIntoView();
  void scrollIntoView((boolean or object) arg);

precisely because the spec wants scrollIntoView() and scrollIntoView({}) to have different behavior, which is not something dictionaries normally do.

Porting Bug 1389274 should fix this issue.

@wolfbeast

This comment has been minimized.

Copy link
Member

wolfbeast commented Jan 8, 2019

I'll poke at this myself anyway.

@wolfbeast wolfbeast closed this in c0a05ad Jan 8, 2019

@wolfbeast wolfbeast changed the title Align Element.ScrillIntoView with the latest draft spec Align Element.ScrollIntoView with the latest draft spec Jan 8, 2019

@wolfbeast wolfbeast added this to the 28.3.0 milestone Jan 8, 2019

wolfbeast added a commit that referenced this issue Jan 8, 2019

Align Element.ScrollIntoView() with the spec. (uplift)
This also removes the (unused) shadow alias from nsIDOMHTMLElement
which used the different calling convention.

This resolves #927
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment