Closed
Bug 852967
Opened 12 years ago
Closed 12 years ago
Simplify marker selection in text inputs
Categories
(Firefox for Metro Graveyard :: Input, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 22
People
(Reporter: jimm, Assigned: jimm)
References
Details
(Whiteboard: [selection])
Attachments
(2 files)
|
2.28 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
|
9.48 KB,
patch
|
Details | Diff | Splinter Review |
Spun off from bug 848594. This solves bug 852089 for text inputs.
| Assignee | ||
Updated•12 years ago
|
Attachment #727204 -
Attachment description: use selection idexes v.1 → use selection indexes v.1
| Assignee | ||
Comment 1•12 years ago
|
||
Comment on attachment 727204 [details] [diff] [review]
use selection indexes v.1
Replacing 16 lines of code with 2!
Attachment #727204 -
Flags: review?(mbrubeck)
| Assignee | ||
Updated•12 years ago
|
Comment 2•12 years ago
|
||
Comment on attachment 727204 [details] [diff] [review]
use selection indexes v.1
Review of attachment 727204 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/metro/base/content/contenthandlers/SelectionHandler.js
@@ +800,5 @@
>
> /*
> * Selection control call wrapper
> */
> _addEditStartSelection: function _addEditStartSelection() {
Could you give both of these functions better descriptive comments? A one-liner would be fine. I just had trouble figuring out what they were for.
Attachment #727204 -
Flags: review?(mbrubeck) → review+
| Assignee | ||
Comment 3•12 years ago
|
||
absolutely, will do.
| Assignee | ||
Comment 4•12 years ago
|
||
I coalesced those two into a single call that takes a param. Also did some additional code re-org and zapped an unused method.
Assignee: nobody → jmathies
| Assignee | ||
Updated•12 years ago
|
Whiteboard: [selection]
| Assignee | ||
Comment 5•12 years ago
|
||
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Updated•11 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•