Closed Bug 1504592 Opened 2 years ago Closed 2 years ago

Slow "left arrow" and "right arrow" keys in a textarea with resize:none; and empty first line.

Categories

(Core :: Web Painting, defect, P1)

61 Branch
Unspecified
All
defect

Tracking

()

VERIFIED FIXED
mozilla65
Tracking Status
firefox-esr60 --- disabled
firefox63 --- wontfix
firefox64 --- verified
firefox65 --- verified

People

(Reporter: serge, Assigned: miko)

References

(Depends on 1 open bug)

Details

(Keywords: perf, regression, testcase)

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

This html:

<!DOCTYPE html>
<html>
<body>
<textarea style="resize:none;width:20em;height:10em">

Slow "left arrow" and "right arrow" keys in a textarea with resize:none; and empty first line.
</textarea>
</body>
</html>


FF: 63.0.1 (64-bit)/Linux



Actual results:

Slow "left arrow" and "right arrow" keys.
Only happens when style has resize:none; AND first line is empty




Expected results:

Normal speed "left arrow" and "right arrow" keys
Attached file reporter's testcase
Attachment #9022523 - Attachment mime type: text/plain → text/html
i can reproduce the issue on Nightly65.0qa1 Windows10.
Status: UNCONFIRMED → NEW
Component: Untriaged → Editor
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Version: 63 Branch → 61 Branch
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7fb001f811d183095e9cfdd958c9215484377396&tochange=d5e1ed773fefe2eb9d52402dd74b5fea19b8b4a4

Regressed by: Bug 1416055

I confirmed that setting layout.display-list.retain to false fixed the issue on the bad build.

@:mattwoodrow,

"retained display lists" causes the regression, can you please look into this?
Blocks: 1416055
Component: Editor → Web Painting
Flags: needinfo?(matt.woodrow)
Priority: -- → P1
The cause for this bug seems to be that we mark the wrong frame modified[1]. Instead of marking the line where the caret actually is (nsContinuingTextFrame), the first empty line (nsTextFrame) is marked. Since that frame has no area, the display list building rect will be empty.


[1]: https://searchfox.org/mozilla-central/rev/6e0e603f4852b8e571e5b8ae133e772b18b6016e/layout/base/nsCaret.cpp#453
Assignee: nobody → mikokm
Status: NEW → ASSIGNED
This patch changes nsCaret::SchedulePaint() logic so that the frame that is marked modified should be the same frame that caret uses for painting.
My assumption is that the previous version was not using the exact frame for performance reasons. If this turns out to be a problem, we can probably cache the frame without calculating it every time.
Flags: needinfo?(matt.woodrow)
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ed759d2a9eff
Mark the right frame modified when caret changes r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/ed759d2a9eff
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Is this something we should be uplifting to 64?
Flags: needinfo?(mikokm)
(In reply to Julien Cristau [:jcristau] from comment #9)
> Is this something we should be uplifting to 64?

Sure, this should be simple enough change.
Flags: needinfo?(mikokm)
Comment on attachment 9023597 [details]
Bug 1504592 - Mark the right frame modified when caret changes

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1416055

User impact if declined: Keyboard navigation in multiline text fields with empty first line will seem slow

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 1) Open the attached testcase
2) Move the caret in the text field with arrow keys. The movement should be fast.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch is simple and only affects the repainting interval of the caret

String changes made/needed: N/A
Attachment #9023597 - Flags: approval-mozilla-beta?
Comment on attachment 9023597 [details]
Bug 1504592 - Mark the right frame modified when caret changes

rdl fix, approved for 64.0b10
Attachment #9023597 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have managed to reproduce this issue on an affected Firefox 65.0a1 (20181104220100) build using Windows 10 x64  by following the STR from comment 0. 

This issue is verified fixed using Firefox 64.0b10 and 65.0a1 (20181115223444) on the following OSes: Windows 10 x64, Ubuntu 16.04 x64, macOS 10.14.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.