1659276 - Window.find() with scroll(0,0) doesn't jump to top of page?

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Matt]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0

Steps to reproduce:

Go to: https://polishforums.com/index.php?phrase=poland+rent&action=search&searchGo=1 and then click on the first result (or any result that has enough posts). The search saves the searched keywords ("Poland Rent" or whatever else you search for) as a local storage item.

Actual results:

You jump to the middle/bottom of the page instead of staying on top. They use scroll(0,0) after the window.wind() and it should scroll up but it doesn't?

Expected results:

You should stay on top of the page. It works this way in all other web browsers (I know window.find is not standard but still). I think it worked correctly in the previous Firefox versions.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a89ab01aec47fc6f457ddf6bfd2650e407f169d3&tochange=5511edd700f79dd849bc3a769e8a5e38dd78745f

Comment 2

[Alice0775 White]

Attachment: reduced html

Open attached

Actual results:
Page scrolled

Expected results:
Page should be top

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't have what's going on with my landing for bug 970802 because of most part of it is still disabled by default... But I should check this when I have much time.

Comment 4

[Alice0775 White]

Attachment: another html

select node, execCommand, and then scroll() would not work.

Comment 5

[Matt]

Comment on attachment 9170606 [details]
another html

I'm still taken to the bottom of the page with this..

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, this is regression of the part 2 of the patches.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Got it. SelectionSelectionIntoView() is newly called by EditorBase::EndPlaceholderTransaction() at end of handling an edit action. Then, it'll be performed asynchronously, but it's not canceled when JS API updates the scroll position. So, this must be a long standing bug and the style-change-edit-action is involved into this path by the patch.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The site does interesting approach for doing it. It seems that "find in page" performance may be more important than I've thought.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sigh, making editor stop using async scroll-selection-into-view request, it causes various timing changes. So, really risky to do it.

Comment 10

[Matt]

Is there a way to make it work today? :)

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Matt from comment #10)

Is there a way to make it work today? :)

If you're the author, I guess that calling scroll with setTimeout avoids this bug. But there is no way from user side.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1659276 - Make `AutoPlaceholderBatch` choose whether do or do not `ScrollSelectionIntoView` r=m_kato!

The root cause of this bug is a bug of async ScrollSelectionIntoView that
is what it won't be canceled even if web app changes scroll position with
any API. Fixing it is really risky and this bug affects an existing website.
Therefore, this patch makes the constructors of AutoPlaceholderBatch take
whether do or do not ScrollSelectionIntoView when it's destructed. And
makes HTMLEditor::SetInlinePropertyAsAction() not request
ScrollSelectionIntoView for taking back the traditional behavior.

Note that this patch does not make it an optional argument because it's hard to
guess that it'll run ScrollSelectionIntoView automatically.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a67d5eac3843
Make `AutoPlaceholderBatch` choose whether do or do not `ScrollSelectionIntoView` r=m_kato

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a67d5eac3843