Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#17740 closed defect (fixed)

dojo/window#scrollIntoView() broken on iOS on RTL page

Reported by: bill Owned by: Brandon Payton <brandon@…>
Priority: undecided Milestone: 1.7.6
Component: Core Version: 1.9.2
Keywords: Cc:
Blocked By: Blocking:

Description

window.scrollIntoView() will scroll the page far to the left, even when it doesn't need to. Happens on RTL document on iOS, but works on chrome.

See attached test case:

  1. load page on iPad in portrait mode
  2. scroll to right
  3. press scroll "node" into view button

Since node is already in view, nothing should happen. Yet, the screen scrolls far to the left, vanishing all the text.

Attachments (1)

scroll.html (740 bytes) - added by bill 6 years ago.
test case showing problem

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by bill

Attachment: scroll.html added

test case showing problem

comment:1 Changed 6 years ago by Brandon Payton

I have observed this issue more generally without an RTL doc. It appears to be caused by iOS resetting body.scrollTop when body.scrollLeft is set and vice versa. I plan to submit a pull request for a workaround in dojo/window tomorrow.

comment:2 Changed 6 years ago by Brandon Payton

I found an issue in the workaround and fixed it, but I am now seeing some rtl-related test failures. I plan to work on this but will likely not get back to it today.

The basic idea of the workaround is to use the scrollBy method on the parent window when scrolling a body. This causes the scroll.html test case to work as expected. I am interested to hear if anyone has concerns about this approach.

comment:3 Changed 6 years ago by bill

Sounds interesting. Not sure if scrollTo() or scrollBy() is easier. Seems like it would be easier to convert the current code to use scrollTo(). But probably you know something I don't.

comment:4 Changed 6 years ago by Brandon Payton

I submitted a PR to fix this: https://github.com/dojo/dojo/pull/66

It addresses the reduced test case and does not cause any unit test failures. I wanted to add a unit test for this, but the dojo/window tests are broken on iOS (maybe because newer iOS versions don't respect iframe height set by the host page), and I don't have time to fix them. I might work on that as part of the doh-to-intern conversion.

comment:5 Changed 5 years ago by Brandon Payton <brandon@…>

Owner: set to Brandon Payton <brandon@…>
Resolution: fixed
Status: newclosed

In 87a492b78c9738ae0d0e41fe81230b151803e819/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:6 Changed 5 years ago by Brandon Payton <brandon@…>

In 0b2aa44c726a8b37a1110d766ea99f8b48d433d4/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:7 Changed 5 years ago by Brandon Payton <brandon@…>

In 4d8a85b808a2c2efaf8f0ae904900d101d460c93/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:8 Changed 5 years ago by Brandon Payton <brandon@…>

In 1f3bda74546110fd0e1a7ddbd29a67abe28a6ed6/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:9 Changed 5 years ago by bpayton

I backported this fix to the 1.9, 1.8, and 1.7 branches.

comment:10 Changed 5 years ago by bill

Milestone: tbd1.7.6
Note: See TracTickets for help on using tickets.