Closed Bug 1504592 Opened 7 years ago Closed 7 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)

Product:
Core
Component:
Web Painting
Version:
61 Branch
Platform:
Unspecified
All
Type:
defect

Tracking

()

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

People

(Reporter: serge, Assigned: mikokm)

References

(Depends on 1 open bug)

Details

(Keywords: perf, regression, testcase)

Attachments

(2 files)

reporter's testcase
217 bytes, text/html
Details
Bug 1504592 - Mark the right frame modified when caret changes
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
Details | Review
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
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
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
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: 7 years ago
status-firefox65: affected → fixed
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
status-firefox64: fixed → verified
status-firefox65: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: