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

VERIFIED FIXED in Firefox 64

Status

()

P1
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: serge, Assigned: miko)

Tracking

(Depends on: 1 bug, {perf, regression, testcase})

61 Branch
mozilla65
Unspecified
All
perf, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 disabled, firefox63 wontfix, firefox64 verified, firefox65 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 months ago
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

Comment 1

5 months ago
Posted file reporter's testcase

Updated

5 months ago
Attachment #9022523 - Attachment mime type: text/plain → text/html

Comment 2

5 months ago
i can reproduce the issue on Nightly65.0qa1 Windows10.
Status: UNCONFIRMED → NEW
status-firefox63: --- → affected
status-firefox64: --- → affected
status-firefox65: --- → affected
status-firefox-esr60: --- → unaffected
Component: Untriaged → Editor
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
OS: Unspecified → All
Product: Firefox → Core
Version: 63 Branch → 61 Branch

Comment 3

5 months ago
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
status-firefox-esr60: unaffected → disabled
Component: Editor → Web Painting
Flags: needinfo?(matt.woodrow)
Keywords: regressionwindow-wanted → perf
status-firefox63: affected → wontfix
Keywords: testcase
(Assignee)

Updated

4 months ago
Priority: -- → P1
(Assignee)

Comment 4

4 months ago
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
(Assignee)

Comment 6

4 months ago
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)

Comment 7

4 months ago
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ed759d2a9eff
Mark the right frame modified when caret changes r=tnikkel

Comment 8

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ed759d2a9eff
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox65: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Is this something we should be uplifting to 64?
Flags: needinfo?(mikokm)
(Assignee)

Comment 10

4 months ago
(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)
(Assignee)

Comment 11

4 months ago
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+

Comment 13

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/c6949142e640
status-firefox64: affected → fixed
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
status-firefox64: fixed → verified
status-firefox65: fixed → verified
Flags: qe-verify+
Depends on: 1508104
You need to log in before you can comment on or make changes to this bug.