Closed
Bug 1160352
Opened 9 years ago
Closed 9 years ago
Enhance SelectionHandler lazy load
Categories
(Firefox for Android Graveyard :: Text Selection, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: capella, Assigned: capella)
Details
Attachments
(1 file, 1 obsolete file)
4.18 KB,
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
Update browser.js to do a better job of lazy loading SelectionHandler, as identified during GeckoCarets implementation. This should provide a start-up time boost for the duration.
Assignee | ||
Comment 1•9 years ago
|
||
Is there a better/built-in way to check if the lazy load script has/has not yet been loaded?
Attachment #8600140 -
Flags: review?(margaret.leibovic)
Comment 2•9 years ago
|
||
Comment on attachment 8600140 [details] [diff] [review] bug1160352.diff Review of attachment 8600140 [details] [diff] [review]: ----------------------------------------------------------------- Let's not do this boolean flag to keep track of whether or not the script has been loaded. That code doesn't get called at startup, so we shouldn't worry about it, but if we do want to try to fix that, we should do it a different way. ::: mobile/android/chrome/content/SelectionHandler.js @@ -615,5 @@ > - removeAction: function(id) { > - // Update actions list and actionbar UI if active. > - delete this.actions[id]; > - this._updateMenu(); > - }, I think we should keep these as add-on APIs. https://developer.mozilla.org/en-US/docs/Extensions/Mobile/API/SelectionHandler ::: mobile/android/chrome/content/browser.js @@ +5148,5 @@ > + > + // Possibly required after SelectionHandler lazy-loads. > + if (SelectionHandler_loaded) { > + SelectionHandler.subdocumentScrolled(this._scrollableElement); > + } I don't think we should do this this way... If we want to try to improve this logic, I think we should just decouple things to avoid calling SelectionHandler directly. Can we try replacing this with an observer service notification?
Attachment #8600140 -
Flags: review?(margaret.leibovic) → review-
Assignee | ||
Comment 3•9 years ago
|
||
Short and simple then !
Attachment #8600140 -
Attachment is obsolete: true
Attachment #8600261 -
Flags: review?(margaret.leibovic)
Comment 4•9 years ago
|
||
Comment on attachment 8600261 [details] [diff] [review] bug1160352.diff Review of attachment 8600261 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/browser.js @@ -7069,5 @@ > - // POST * YES > - // POST application/x-www-form-urlencoded YES > - // POST text/plain NO ( a little tricky to do) > - // POST multipart/form-data NO > - // POST everything else YES Please keep this comment.
Attachment #8600261 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 5•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a409cbc2582f
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/18347201f035
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•