Closed Bug 1453421 Opened 7 years ago Closed 7 years ago

Find bar shouldn't loop over all event properties, triggering sync reflows from e.g. event.rangeOffset

Categories

(Toolkit :: Find Toolbar, enhancement)

Product:
Toolkit
Component:
Find Toolbar
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Whiteboard: [fxperf:p2])

Attachments

(1 file)

Bug 1453421 - stop findbar from looping over every single property,
59 bytes, text/x-review-board-request
mikedeboer
: review+
Details
As in summary. Instead, we should have a list of properties we actually need in the parent to make decisions. These should be pretty limited (effectively modifier keys, charCode and/or 'key', I expect). Then we just send those properties. That will be faster + won't trigger layout/style flushes.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8968853 [details] Bug 1453421 - stop findbar from looping over every single property, https://reviewboard.mozilla.org/r/237572/#review243300 I put two things down for your consideration. Thanks! ::: toolkit/content/browser-content.js:1077 (Diff revision 1) > _passKeyToParent(event) { > event.preventDefault(); > + // These are the properties required to dispatch another 'real' event > + // to the findbar in the parent in _dispatchKeypressEvent in findbar.xml . > + // If you make changes here, verify that that method can still do its job. > + let requiredProps = [ nit: dunno if it matters already, but s/let/const/ ::: toolkit/content/widgets/findbar.xml:776 (Diff revision 1) > - <parameter name="aEvent"/> > + <parameter name="fakeEvent"/> > <body><![CDATA[ > if (!aTarget) > return; > > let event = document.createEvent("KeyboardEvent"); nit: while you're here, maybe you'd like to modernize this to use KeyboardEvent instead? ```js const global = aTarget.ownerGlobal; aTarget.dispatchEvent(new global.KeyboardEvent(fakeEvent.type, fakeEvent)); ```
Attachment #8968853 - Flags: review?(mdeboer) → review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/697d0f7076eb stop findbar from looping over every single property, r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/697d0f7076eb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: