Closed Bug 1659276 Opened 5 months ago Closed 5 months ago

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

Categories

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

79 Branch
defect

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- fixed

People

(Reporter: tomaszattomasz, Assigned: masayuki)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

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.

Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → DOM: Editor
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Regressed by: 970802
Attached file reduced html

Open attached

Actual results:
Page scrolled

Expected results:
Page should be top

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.

Severity: -- → S3
Priority: -- → P3
Attached file another html

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

Comment on attachment 9170606 [details]
another html

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

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.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

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

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

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

(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.

Root Cause: --- → Coding: Internal API Issue

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.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/a67d5eac3843
Make `AutoPlaceholderBatch` choose whether do or do not `ScrollSelectionIntoView` r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.