"element.scrollIntoView()" uses invalid scrolling behavior of "instant"
Categories
(Remote Protocol :: Marionette, defect, P3)
Tracking
(firefox97 fixed)
Tracking | Status | |
---|---|---|
firefox97 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
()
Details
Attachments
(2 files)
As noticed yesterday in some cases the method element.scrollIntoView
returns too early, and as such the element is not yet visible inside the viewport. In case of taking screenshots the wrong part of the web page will be taken.
As figured out yesterday the used behavior instant
is not actually valid, and as such it falls back to auto
, which isn't really instant.
Anyway, as just noticed now I also get an assertion when manually calling scrollIntoView()
inside the developer tools on that specific page, and element.isInView()
also returns true
too early.
Lets use this bug as meta bug, and I will file blocking bugs for whatever issue I find including the assertion above.
Assignee | ||
Comment 1•5 years ago
|
||
Here a Marionette test which demonstrates that a different area than from the specified element is taken a screenshot from. As of now I can only make this work when adding an additional sleep of more than 18ms after the call to scrollIntoView()
.
Note that for this case the above mentioned assertion doesn't occur. So that might have been a red herring.
Botond, you mentioned something on IRC yesterday, maybe you can summarize here on the bug again? What we basically want is a behavior of instant
scrolling and not any other kind of smooth
.
Comment 2•5 years ago
|
||
Are you trying to scroll the target element into view across cross-origin document boundaries? It doesn't synchronously happen unfortunately. So, if you are sure that the element causes scroll for the parent document, you can check scroll
event just like this.
If you are unsure whether scrolling happens or not, I don't have any clear ideas right now. We can maybe add a notification which is sent when it finished.
Assignee | ||
Comment 3•5 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
Are you trying to scroll the target element into view across cross-origin document boundaries? It doesn't synchronously happen unfortunately.
No, it's all within the same page, just an element located outside the current viewport. Means in Marionette itself it is done within a framescript, but maybe that causes the behavior as you mentioned above?
So, if you are sure that the element causes scroll for the parent document, you can check
scroll
event just like this.
That actually works great! Can I assume it is always only a single scroll
event which gets fired, and indicates that the scrolling is done? It's not that clear for me when reading the spec.
If you are unsure whether scrolling happens or not, I don't have any clear ideas right now. We can maybe add a notification which is sent when it finished.
You mean it would be sent out nevertheless if a scroll actually happened or not? If that's possible fine, otherwise we could check if the element isn't in the current viewport, and only then call scrollIntoView()
.
Comment 4•5 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #1)
Botond, you mentioned something on IRC yesterday, maybe you can summarize here on the bug again? What we basically want is a behavior of
instant
scrolling and not any other kind ofsmooth
.
Since scrollIntoView()
only supports the behaviours smooth
and auto
, and for us auto
also defaults to smooth (depending on the general.smoothScroll
pref), if you want instant scrolling in a test you probably need to do it using an internal API.
PresShell::ScrollContentIntoView()
supports instant scrolling (if you pass it neither ScrollSmooth
nor ScrollSmoothAuto
in its aScrollFlags
argument), and it could be exposed to JS via nsIDOMWindowUtils
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #3)
Can I assume it is always only a single
scroll
event which gets fired, and indicates that the scrolling is done?
I don't think that's guaranteed. I think in the case of smooth scrolling you can get multiple scroll events as the smooth scroll progresses.
Assignee | ||
Comment 5•5 years ago
|
||
See also https://github.com/w3c/webdriver/issues/1449 for a change to the webdriver spec, which is about replacing instant
with smooth
.
Comment 6•3 years ago
|
||
The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:whimboo, maybe it's time to close this bug?
Assignee | ||
Comment 7•3 years ago
|
||
We will have to re-check if it is still needed, but so far no need for a meta bug.
Assignee | ||
Comment 8•2 years ago
|
||
I tried to reproduce the failure with the given test case from comment 1 but I'm not able to anymore. As such I would say we just remove the invalid instant
behavior and call the bug done.
Assignee | ||
Comment 9•2 years ago
|
||
Updated•2 years ago
|
Comment 10•2 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/979d4e44b30d [marionette] Remove invalid "instant" behavior for scrollIntoView(). r=webdriver-reviewers,jdescottes
Comment 11•2 years ago
|
||
bugherder |
Updated•1 year ago
|
Description
•