Implement alternative to CSSOM-View scroll-behavior Element.scrollTop and Element.scrollLeft extensions

RESOLVED FIXED in mozilla36

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: kip, Assigned: kip)

Tracking

(Blocks 1 bug, {dev-doc-needed})

Trunk
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(2 attachments, 6 obsolete attachments)

Assignee

Description

5 years ago
The CSSOM-View scroll-behavior specification includes invalid WebIDL for the extensions to the Element.scrollTop and Element.scrollLeft attributes:

http://dev.w3.org/csswg/cssom-view/#extensions-to-the-element-interface

A bug has been filed on w3c.org to request that the WebIDL be changed for the extensions:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=26294

Discussion has started on www-style on the issue:

http://lists.w3.org/Archives/Public/www-style/2014Jul/0385.html

Once the alternative WebIDL has been selected, this bug will track implementation of the alternative Element DOM extensions.

The other CSSOM-View scroll-behavior DOM methods can be implemented without being blocked by this.  Bug 1022818 tracks the implementation of the rest of the CSSOM-View scroll-behavior DOM methods.
Blocks: 1033468
Assignee

Comment 1

5 years ago
Comments on the w3.org mailing list have requested that new scrolling functions include support for abstract mapping of scroll offsets sensitive to writing modes.

I propose an extension to the Element interface, which we could implement partial support for now, without requiring the new function to be deprecated when abstract mapping of scroll offsets could be added in the future...

.
.
.

In order to include writing-mode and rtl agnostic scroll offset support, I propose that
we create a new function, Element.scrollTo, which takes a dictionary derived from
ScrollOptions.

Proposed WebIDL
---------------

dictionary ElementScrollOptions : ScrollOptions {
  double? left;
  double? top;
  double? right;
  double? bottom;
  double? blockStart;
  double? blockEnd;
  double? inlineStart;
  double? inlineEnd;
};

void scrollTo(optional ElementScrollOptions options);

Members
-------

Various combinations of ElementScrollOptions members can be passed to scrollTo to
select the scroll destination with a physical or abstract offset:

left:
	- Number of pixels that an element’s content is scrolled to the left.  Equivalent
	  to setting the "Element.scrollLeft" attribute.
	- If the direction of the element is ltr (left-to-right) then "left" is 0 when the
	  scrollbar is at its leftmost position (at start of the scrolled content) and then
	  increases as you scroll towards the end of the content.
	- If the direction of the element is rtl (right-to-left) then "left" is 0 when the
	  scrollbar is at its rightmost position (at start of the scrolled content) and
	  then increasingly negative as you scroll towards the end of the content.

right:
	- Number of pixels that an element’s content is scrolled to the right.
	- If the direction of the element is ltr (left-to-right) then "right" is 0 when the
	  scrollbar is at its rightmost position (at the end of the scrolled content) and
	  then increasingly negative as you scroll towards the start of the content.
	- If the direction of the element is rtl (right-to-left) then "right" is 0 when the
	  scrollbar is at its leftmost position (at the end of the scrolled content) and then
	  increases as you scroll towards the start of the content.

top:
	- Number of pixels that the content of an element is scrolled upward.  Equivalent
	  to setting the "Element.scrollLeft" attribute.
	- "top" is 0 when the scrollbar is at its topmost position (at the start of the
	   scrolled content) and then increases as you scroll towards the bottom of the
	   content.

bottom:
	- Number of pixels that the content of an element is scrolled downward.
  - "bottom" is 0 when the scrollbar is at its bottommost position (at the start of
    the scrolled content) and then increasingly negative as you scroll towards the top
    of the content.

blockStart:
	- "blockStart" is an offset relative to the side that comes earlier in the block
	  progression, as determined by the writing-mode property.
	- In horizontal-tb mode, is equivalent to setting value of "top".
	- In vertical-rl mode, is equivalent to setting value of "right".
	- In vertical-lr mode, is equivalent to setting value of "left".

inlineStart:
	- "inlineStart" is an offset relative to the side from which text of the inline
	  base direction would start.
  - With a direction value of ltr, this is the line-left side.
  - With a direction value of rtl, this is the line-right side.

blockEnd:
	- "blockEnd" is an offset is relative to the side opposite block-start.

inlineEnd:
	- "inlineEnd" is an offset is relative to the side opposite inline-start.

For a complete list of all combinations and their equivalent physical mappings, see [2].

Rules
-----
- "left", "top", "right", and "bottom" represent physical scrolling offsets.
- "blockStart", "blockEnd", "inlineStart", and "inlineEnd" represent offsets
  in flow-relative directions [1].
- Values assigned to "blockStart", "blockEnd", "inlineStart", and "inlineEnd" are set
  to the equivalent physical offsets of left, top, right, and bottom as per
  "6.4 Abstract-to-Physical Mappings" in the "CSS Writing Modes Level 3"
  specification [2].
- If physical scrolling offsets are mixed with offsets in flow-relative directions, an
  exception will be thrown.
- If non-finite values (NaN, -Infinity, Infinity) are passed to any member of
  ElementScrollOptions, an exception will be thrown.
- The scrolling offset is considered over-constrained if more than one offset for any
  axis are passed simultaneously.
  - "left" and "right" are mutually exclusive
  - "top" and "bottom" are mutually exclusive
  - "blockStart" and "blockEnd" are mutually exclusive
  - "inlineStart" and "inlineEnd" are mutually exclusive
- If the scrolling offset is over-constrained, an exception will be thrown.
- If a scrolling offset for an axis is not passed, then the position of that axis
  at the start of the scroll will be used as the destination position for that axis.
- If no scrolling offsets are included in ElementScrollOptions, the scrollTo call
  will perform a scroll with the destination set to the current scroll offset.  This may
  cause a smooth scroll [3] to be interrupted.  If ScrollOptions and the smooth-scroll
  CSS property evaluate to a smooth scroll, then a smooth scroll to the current position
  is performed to end the scroll at the specified offset in a UA defined smooth fashion.


[1] CSS Writing Modes Level 3 - Flow-relative directions
http://dev.w3.org/csswg/css-writing-modes/#block-start

[2] CSS Writing Modes Level 3 - 6.4 Abstract-to-Physical Mappings 
http://dev.w3.org/csswg/css-writing-modes/#logical-to-physical

[3] CSSOM View Module - 4.1 Scrolling
http://dev.w3.org/csswg/cssom-view/#scrolling
Assignee

Comment 2

5 years ago
(In reply to :kip (Kearwood Gilbert) from comment #1)
> Comments on the w3.org mailing list have requested that new scrolling
> functions include support for abstract mapping of scroll offsets sensitive
> to writing modes.
> 
> I propose an extension to the Element interface, which we could implement
> partial support for now, without requiring the new function to be deprecated
> when abstract mapping of scroll offsets could be added in the future...
> 

Hello Boris,

Would you like to give any feedback on this approach before forwarding this to the w3.org mailing list?

Essentially, I would like to solve our immediate scroll-behavior needs and address the feedback on the w3.org mailing list by implementing a Element.scrollTo method that takes a dictionary with left and top members instead of separate left and top arguments.  Hopefully, this will allow this function to be extended in a manner such as my proposal for support of abstract mapping.  My goal is to avoid future deprecation or forward-compatibility issues with a new element.scrollTo method.
Flags: needinfo?(bzbarsky)
Why are all those members nullable?  What happens if both left and right are set in the dictionary?

Past that, you should probably run this by roc; he's though mode about scrolling than I have.
Flags: needinfo?(bzbarsky)
No longer blocks: 1033468
Assignee

Comment 4

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #3)
> Why are all those members nullable?  What happens if both left and right are
> set in the dictionary?
> 
> Past that, you should probably run this by roc; he's though mode about
> scrolling than I have.
The intent is to allow the scroll offset to be specified using either physical (left, right, top, bottom) or logical ( blockStart, blockEnd, inlineStart, inlineEnd) positions.

If a scroll axis is under-constrained, the current position is maintained as the target for that axis.
If a scroll axis is over-constrained, an exception will be thrown by the DOM call.
Assignee

Comment 5

5 years ago
(In reply to :kip (Kearwood Gilbert) from comment #1)
> Comments on the w3.org mailing list have requested that new scrolling
> functions include support for abstract mapping of scroll offsets sensitive
> to writing modes.
> 
> I propose an extension to the Element interface, which we could implement
> partial support for now, without requiring the new function to be deprecated
> when abstract mapping of scroll offsets could be added in the future...
> 
> .
> .
> .
> 
> In order to include writing-mode and rtl agnostic scroll offset support, I
> propose that
> we create a new function, Element.scrollTo, which takes a dictionary derived
> from
> ScrollOptions.
> 
> Proposed WebIDL
> ---------------
> 
> dictionary ElementScrollOptions : ScrollOptions {
>   double? left;
>   double? top;
>   double? right;
>   double? bottom;
>   double? blockStart;
>   double? blockEnd;
>   double? inlineStart;
>   double? inlineEnd;
> };
> 
> void scrollTo(optional ElementScrollOptions options);
> 
> Members
> -------
> 
> Various combinations of ElementScrollOptions members can be passed to
> scrollTo to
> select the scroll destination with a physical or abstract offset:
> 
> left:
> 	- Number of pixels that an element’s content is scrolled to the left. 
> Equivalent
> 	  to setting the "Element.scrollLeft" attribute.
> 	- If the direction of the element is ltr (left-to-right) then "left" is 0
> when the
> 	  scrollbar is at its leftmost position (at start of the scrolled content)
> and then
> 	  increases as you scroll towards the end of the content.
> 	- If the direction of the element is rtl (right-to-left) then "left" is 0
> when the
> 	  scrollbar is at its rightmost position (at start of the scrolled content)
> and
> 	  then increasingly negative as you scroll towards the end of the content.
> 
> right:
> 	- Number of pixels that an element’s content is scrolled to the right.
> 	- If the direction of the element is ltr (left-to-right) then "right" is 0
> when the
> 	  scrollbar is at its rightmost position (at the end of the scrolled
> content) and
> 	  then increasingly negative as you scroll towards the start of the content.
> 	- If the direction of the element is rtl (right-to-left) then "right" is 0
> when the
> 	  scrollbar is at its leftmost position (at the end of the scrolled
> content) and then
> 	  increases as you scroll towards the start of the content.
> 
> top:
> 	- Number of pixels that the content of an element is scrolled upward. 
> Equivalent
> 	  to setting the "Element.scrollLeft" attribute.
> 	- "top" is 0 when the scrollbar is at its topmost position (at the start of
> the
> 	   scrolled content) and then increases as you scroll towards the bottom of
> the
> 	   content.
> 
> bottom:
> 	- Number of pixels that the content of an element is scrolled downward.
>   - "bottom" is 0 when the scrollbar is at its bottommost position (at the
> start of
>     the scrolled content) and then increasingly negative as you scroll
> towards the top
>     of the content.
> 
> blockStart:
> 	- "blockStart" is an offset relative to the side that comes earlier in the
> block
> 	  progression, as determined by the writing-mode property.
> 	- In horizontal-tb mode, is equivalent to setting value of "top".
> 	- In vertical-rl mode, is equivalent to setting value of "right".
> 	- In vertical-lr mode, is equivalent to setting value of "left".
> 
> inlineStart:
> 	- "inlineStart" is an offset relative to the side from which text of the
> inline
> 	  base direction would start.
>   - With a direction value of ltr, this is the line-left side.
>   - With a direction value of rtl, this is the line-right side.
> 
> blockEnd:
> 	- "blockEnd" is an offset is relative to the side opposite block-start.
> 
> inlineEnd:
> 	- "inlineEnd" is an offset is relative to the side opposite inline-start.
> 
> For a complete list of all combinations and their equivalent physical
> mappings, see [2].
> 
> Rules
> -----
> - "left", "top", "right", and "bottom" represent physical scrolling offsets.
> - "blockStart", "blockEnd", "inlineStart", and "inlineEnd" represent offsets
>   in flow-relative directions [1].
> - Values assigned to "blockStart", "blockEnd", "inlineStart", and
> "inlineEnd" are set
>   to the equivalent physical offsets of left, top, right, and bottom as per
>   "6.4 Abstract-to-Physical Mappings" in the "CSS Writing Modes Level 3"
>   specification [2].
> - If physical scrolling offsets are mixed with offsets in flow-relative
> directions, an
>   exception will be thrown.
> - If non-finite values (NaN, -Infinity, Infinity) are passed to any member of
>   ElementScrollOptions, an exception will be thrown.
> - The scrolling offset is considered over-constrained if more than one
> offset for any
>   axis are passed simultaneously.
>   - "left" and "right" are mutually exclusive
>   - "top" and "bottom" are mutually exclusive
>   - "blockStart" and "blockEnd" are mutually exclusive
>   - "inlineStart" and "inlineEnd" are mutually exclusive
> - If the scrolling offset is over-constrained, an exception will be thrown.
> - If a scrolling offset for an axis is not passed, then the position of that
> axis
>   at the start of the scroll will be used as the destination position for
> that axis.
> - If no scrolling offsets are included in ElementScrollOptions, the scrollTo
> call
>   will perform a scroll with the destination set to the current scroll
> offset.  This may
>   cause a smooth scroll [3] to be interrupted.  If ScrollOptions and the
> smooth-scroll
>   CSS property evaluate to a smooth scroll, then a smooth scroll to the
> current position
>   is performed to end the scroll at the specified offset in a UA defined
> smooth fashion.
> 
> 
> [1] CSS Writing Modes Level 3 - Flow-relative directions
> http://dev.w3.org/csswg/css-writing-modes/#block-start
> 
> [2] CSS Writing Modes Level 3 - 6.4 Abstract-to-Physical Mappings 
> http://dev.w3.org/csswg/css-writing-modes/#logical-to-physical
> 
> [3] CSSOM View Module - 4.1 Scrolling
> http://dev.w3.org/csswg/cssom-view/#scrolling

Hello Roc,

Would you be able to add some feedback on this proposed DOM / WebIDL change?  The intent is to ensure that our immediately needed change will not need to be deprecated when functionality requested in the feedback on the w3.org forum is implemented in the future.  I intend to land the dictionary parameter with support for "scrollLeft" and "scrollTop" members only for now.
Flags: needinfo?(roc)
> The intent is to allow the scroll offset to be specified using either ...

OK, and answers the second part of my question, but why does this involve nullable members?
I think Boris is saying that it's enough for the members to be optional (which they implicitly are, since they're in a dictionary).
Flags: needinfo?(roc)
This basically looks right to me if you just remove all the nullable annotations.

Also I think if someone passes multiple values for the same axis I think we should throw instead of ignoring that axis.
Assignee

Comment 9

5 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> This basically looks right to me if you just remove all the nullable
> annotations.
> 
> Also I think if someone passes multiple values for the same axis I think we
> should throw instead of ignoring that axis.

Thanks Roc,

I'll remove the nullable annotations.  Also, I agree on throwing when multiple values are assigned for the same axis.  I'll propose this on the w3.org forum and bug linked above.
Assignee

Comment 10

5 years ago
The CSSOM-View specification has been updated with new WebIDL for the extensions to the Window interface and the Element interface:

http://dev.w3.org/csswg/cssom-view/#extensions-to-the-window-interface
http://dev.w3.org/csswg/cssom-view/#extensions-to-the-element-interface

Due to the use of dictionaries for the parameters, it will be possible to land the smooth scroll equivalent of setting scrollLeft and scrollTop, while allowing future implementation of the logical axis versions.

This bug will include updates to both the Window interface and the Element interface.
Assignee

Comment 11

5 years ago
A bug has been filed on w3.org requesting clarification on the behavior of chaining smooth scrolls together and whether a new smooth scroll with only one axis specified in a dictionary should abort an existing smooth scroll that is animating on the other axis.

I have responded with a proposal that matches our implementation's inherent behavior for smooth scroll transitions (aborting existing smooth scroll):

https://www.w3.org/Bugs/Public/show_bug.cgi?id=26909

Additionally, this raised the issue of how to programmatically abort a smooth scroll.  I have re-iterated the recommendation I made above, in Comment 5, which describes a mechanism for stopping smooth scrolls by specifying no axis positions in the ScrollToOptions dictionary.
Assignee

Comment 12

5 years ago
(In reply to :kip (Kearwood Gilbert) from comment #11)
> A bug has been filed on w3.org requesting clarification on the behavior of
> chaining smooth scrolls together and whether a new smooth scroll with only
> one axis specified in a dictionary should abort an existing smooth scroll
> that is animating on the other axis.
> 
> I have responded with a proposal that matches our implementation's inherent
> behavior for smooth scroll transitions (aborting existing smooth scroll):
> 
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=26909
> 
> Additionally, this raised the issue of how to programmatically abort a
> smooth scroll.  I have re-iterated the recommendation I made above, in
> Comment 5, which describes a mechanism for stopping smooth scrolls by
> specifying no axis positions in the ScrollToOptions dictionary.
My response on the w3.org bug was accepted and the bug has been closed.  I am beginning implementation of the WebIDL changes.
Assignee

Comment 13

5 years ago
- Added new WebIDL dictionary, ScrollToOptions.  This dictionary extends
  ScrollOptions by adding "left" and "top", specifying the scroll offset.
  This will be later extended with more members to allow scroll offsets to be
  specified with logical axes.
- Implemented Window.Scroll, Window.ScrollTo, Window.ScrollBy, Element.Scroll,
  Element.ScrollTo, and Element.ScrollBy functions that accept ScrollToOptions
  as a single parameter.
- Removed ScrollOptions dictionary parameter from existing Window.Scroll,
  Window.ScrollTo, and Window.ScrollBy functions as these have been replaced
  with functions accepting a single parameter, ScrollToOptions.

Note: WORK IN PROGRESS - Currently crashes with e10s and tests need updating
Assignee

Comment 14

5 years ago
(In reply to :kip (Kearwood Gilbert) from comment #13)
> Note: WORK IN PROGRESS - Currently crashes with e10s and tests need updating
e10s related crash was related to a local configuration issue and occurred without this patch as well.  There are no known issues with the patch.  Tests are to follow.
Assignee

Comment 15

5 years ago
Posted patch Bug 1045754: Part 2 - Tests (obsolete) — Splinter Review
- Updated smooth scroll behavior mochitest (test_scroll_behavior.html) to
match new CSSOM-View DOM scrolling method parameters.

Note - WORK IN PROGRESS - Additional tests will be added to include new CSSOM-View DOM methods added by the part 1 patch.
Assignee

Updated

5 years ago
Depends on: 1010538
No longer depends on: 1022818
Assignee

Comment 16

5 years ago
These patches have been re-based on top of the patches from Bug 1010538.  Please apply Bug 1010538 patches first.
Assignee

Updated

5 years ago
Attachment #8499233 - Attachment description: Bug 1045754 - Implement updated CSSOM-View smooth-scrolling specification → Bug 1045754: Part 1 - Implement updated CSSOM-View smooth-scrolling specification
Assignee

Comment 17

5 years ago
- Added new WebIDL dictionary, ScrollToOptions. This dictionary extends
ScrollOptions by adding "left" and "top", specifying the scroll offset.
This will be later extended with more members to allow scroll offsets to be
specified with logical axes.
- Implemented Window.Scroll, Window.ScrollTo, Window.ScrollBy, Element.Scroll,
Element.ScrollTo, and Element.ScrollBy functions that accept ScrollToOptions
as a single parameter.
- Removed ScrollOptions dictionary parameter from existing Window.Scroll,
Window.ScrollTo, and Window.ScrollBy functions as these have been replaced
with functions accepting a single parameter, ScrollToOptions.

V2 Patch:
- Added new WebIDL dictionary, ScrollIntoViewOptions. This dictionary
extends ScrollOptions by adding "block", specifying whether the element
start or end will be scrolled into view.
- Replaced Element.ScrollIntoView(bool,ScrollOptions) with
Element.ScrollIntoView(ScrollIntoViewOptions) to match updated
CSSOM-View scroll-behavior specification.
Attachment #8499233 - Attachment is obsolete: true
Assignee

Comment 18

5 years ago
Posted patch Bug 1045754: Part 2 - Tests (obsolete) — Splinter Review
- Updated smooth scroll behavior mochitest and reftests to
match new CSSOM-View DOM scrolling method parameters.

V2 Patch:
- Added tests for Element.ScrollBy and Element.ScrollTo.
- Scroll-behavior reftests updated to use ScrollToOptions
  and ScrollIntoViewOptions dictionaries.
Attachment #8499277 - Attachment is obsolete: true
Assignee

Comment 19

5 years ago
Comment on attachment 8501411 [details] [diff] [review]
Bug 1045754: Part 1 - Implement updated CSSOM-View smooth-scrolling specification (V2 Patch)

Hi BZ, would you have some extra cycles to give this a review?  I aim to land this for 34.  Please advise if you are too busy.

These patches require the patchset from Bug 1010538 to be applied first.

Thanks!
Attachment #8501411 - Flags: review?(bzbarsky)
Assignee

Comment 20

5 years ago
Comment on attachment 8501414 [details] [diff] [review]
Bug 1045754: Part 2 - Tests

Hi BZ, would you have some extra cycles to give this a review?  I aim to land this for 34.  Please advise if you are too busy.

These patches require the patchset from Bug 1010538 to be applied first.

Thanks!
Attachment #8501414 - Flags: review?(bzbarsky)
Comment on attachment 8501411 [details] [diff] [review]
Bug 1045754: Part 1 - Implement updated CSSOM-View smooth-scrolling specification (V2 Patch)

>+#include "mozilla/dom/ElementBinding.h"

I would vastly prefer it if we forward-declared the relevant dictionary type and didn't inline the methods using it.  I don't see any particular reason to inline them.

Why do we have both scroll() and scrollTo() if they do the same thing?  Or rather why does the spec?  Ah, well.

Can we have scrollBy call scrollTo/scroll after it figures out where we're scrolling to?  That would avoid all the code duplication...

The spec for scrollBy seems to be buggy; it's talking left/top dictionary members which are not guaranteed to be present.  File a spec issue, please?

r=me with the above fixed.
Attachment #8501411 - Flags: review?(bzbarsky) → review+
Comment on attachment 8501414 [details] [diff] [review]
Bug 1045754: Part 2 - Tests

For the cases that don't pass behavior, don't we want to test both the dictionary and the two numeric arguments (scrollTo/By) or boolean (scrollIntoView)?  Probably by making copies of the existing tests that do that...

r=me with that
Attachment #8501414 - Flags: review?(bzbarsky) → review+
Assignee

Comment 24

5 years ago
- Added new WebIDL dictionary, ScrollToOptions. This dictionary extends
ScrollOptions by adding "left" and "top", specifying the scroll offset.
This will be later extended with more members to allow scroll offsets to be
specified with logical axes.
- Implemented Window.Scroll, Window.ScrollTo, Window.ScrollBy, Element.Scroll,
Element.ScrollTo, and Element.ScrollBy functions that accept ScrollToOptions
as a single parameter.
- Removed ScrollOptions dictionary parameter from existing Window.Scroll,
Window.ScrollTo, and Window.ScrollBy functions as these have been replaced
with functions accepting a single parameter, ScrollToOptions.
- Added new WebIDL dictionary, ScrollIntoViewOptions. This dictionary
extends ScrollOptions by adding "block", specifying whether the element
start or end will be scrolled into view.
- Replaced Element.ScrollIntoView(bool,ScrollOptions) with
Element.ScrollIntoView(ScrollIntoViewOptions) to match updated
CSSOM-View scroll-behavior specification.

V3 Patch:
- Corrected mochitest failures by restoring an inadvertently deleted WebIDL function: Element.ScrollIntoView(bool)
- Updated with feedback from code review in Comment 22
Attachment #8501411 - Attachment is obsolete: true
Assignee

Comment 25

5 years ago
- Updated smooth scroll behavior mochitest and reftests to
match new CSSOM-View DOM scrolling method parameters.
- Added tests for Element.ScrollBy and Element.ScrollTo.

V2 Patch:
- Added tests for Element.ScrollTo(double, double), Element.ScrollBy(double, double), Window.ScrollTo(double, double), Window.ScrollBy(double, double), and Element.ScrollIntoView(bool).  These are duplicates of tests in previous versions of this patch, but modified to exercise the alternate parameter types of the CSSOM-View scrolling DOM Methods
Attachment #8501414 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8502184 - Attachment description: ug 1045754: Part 1 - Implement updated CSSOM-View smooth-scrolling specification (V3 Patch), r=bz → Bug 1045754: Part 1 - Implement updated CSSOM-View smooth-scrolling specification (V3 Patch), r=bz
Assignee

Comment 26

5 years ago
I have pushed this to try, to run the updated reftests on all platforms:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=11870752d814
Assignee

Comment 27

5 years ago
- Added new WebIDL dictionary, ScrollToOptions. This dictionary extends
ScrollOptions by adding "left" and "top", specifying the scroll offset.
This will be later extended with more members to allow scroll offsets to be
specified with logical axes.
- Implemented Window.Scroll, Window.ScrollTo, Window.ScrollBy, Element.Scroll,
Element.ScrollTo, and Element.ScrollBy functions that accept ScrollToOptions
as a single parameter.
- Removed ScrollOptions dictionary parameter from existing Window.Scroll,
Window.ScrollTo, and Window.ScrollBy functions as these have been replaced
with functions accepting a single parameter, ScrollToOptions.
- Added new WebIDL dictionary, ScrollIntoViewOptions. This dictionary
extends ScrollOptions by adding "block", specifying whether the element
start or end will be scrolled into view.
- Replaced Element.ScrollIntoView(bool,ScrollOptions) with
Element.ScrollIntoView(ScrollIntoViewOptions) to match updated
CSSOM-View scroll-behavior specification.

V4 Patch:
- Un-bitrotted
Attachment #8502184 - Attachment is obsolete: true
Assignee

Comment 28

5 years ago
Pushed the latest version of these patches (and Bug 1010538 patch set) to try:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a5616b2b83a9
Assignee

Comment 29

5 years ago
- Updated smooth scroll behavior mochitest and reftests to
match new CSSOM-View DOM scrolling method parameters.
- Added tests for Element.ScrollBy and Element.ScrollTo.

V3 Patch:
- Un-bitrotted
- Reduced scrolling size from 10000x10000 to 2000x2000 as per Bug 1010538 Comment 108, correcting reftest failures.
Attachment #8502777 - Attachment is obsolete: true
Assignee

Comment 31

5 years ago
This has also been pt-push try'ed:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=290997f11570

If this is green and Bug 1010538 has been checked in, then this can be checked in as well.
Assignee

Updated

5 years ago
Keywords: checkin-needed
Assignee

Comment 32

5 years ago
Try is green.  Please check-in Bug 1010538 followed by this one (Bug 1045754)
https://hg.mozilla.org/mozilla-central/rev/8247fd879542
https://hg.mozilla.org/mozilla-central/rev/5eeef6a33a62
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee

Updated

5 years ago
See Also: → 1089836

Comment 35

4 years ago
As far as I understand, only “left” and “top” out of left/top/right/bottom/{block,inline}{Start,End} are implemented. Is there a bug tracking implementation of the others? `.scrollTo({bottom: 0})` would be very convenient, and is something I look foward being able to do.
Kearwood, do you recall why we made the dictionary members unrestricted here?  The spec has restricted doubles; see <https://lists.w3.org/Archives/Public/www-style/2015Feb/0531.html>.
Flags: needinfo?(kgilbert)
Assignee

Comment 37

4 years ago
(In reply to Not doing reviews right now from comment #36)
> Kearwood, do you recall why we made the dictionary members unrestricted
> here?  The spec has restricted doubles; see
> <https://lists.w3.org/Archives/Public/www-style/2015Feb/0531.html>.
Please see Bug 1062406 Comment 7

This was for backwards compatibility with sites that expected NaN values to be coerced to 0's in calls to ScrollTo/ScrollBy/Scroll.

Perhaps it would be reasonable to implement the dictionary members as restricted, while leaving the legacy methods that don't accept the dictionary members as unrestricted?
Flags: needinfo?(kgilbert)
Yes, I specifically meant the dictionary members; everyone is clear that the legacy double args need to stay unrestricted.
You need to log in before you can comment on or make changes to this bug.