Closed
Bug 1064657
Opened 5 years ago
Closed 5 years ago
Add testSelectionHandler test to ensure readOnly input elements can't be cut
Categories
(Firefox for Android :: Text Selection, defect)
Not set
Tracking
()
RESOLVED
FIXED
Firefox 35
People
(Reporter: capella, Assigned: capella)
References
Details
Attachments
(1 file, 1 obsolete file)
3.36 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
Followup from bug 1060423 comment #22 ... where I let a new contributor slide thinking no-one was paying attention ;-)
Assignee | ||
Comment 1•5 years ago
|
||
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #8486160 -
Flags: review?(wjohnston)
Comment 2•5 years ago
|
||
Comment on attachment 8486160 [details] [diff] [review] bug1064657.diff Review of attachment 8486160 [details] [diff] [review]: ----------------------------------------------------------------- I don't really want to add this check before doing an action. It covers up a bug (that the action was available to you). ::: mobile/android/chrome/content/SelectionHandler.js @@ +122,4 @@ > case "TextSelection:Action": > for (let type in this.actions) { > if (this.actions[type].id == aData) { > + if (this.actions[type].selector.matches(this._targetElement)) { I don't love doing this :( i.e. we shouldn't (and won't) be firing this action if matches was false when we build the list of available actions. We really need some tests that do a bit more on the Java side when the action mode is shown. If you wanted to be tricky here though, you could try stealing Messaging now and then intercepting the call to _updateMenu. i.e. return new Promise((resolve, reject) => { let oldMessaging = Messaging; Messaging = { sendRequest: function(request) { // check type and actions resolve(); } } sh.startSelection(readOnlyNode); // This will trigger sendRequest above.. maybe more than once. ugh :( Messaging = oldMessaging; }); Not sure if that's better or not :)
Attachment #8486160 -
Flags: review?(wjohnston) → review-
Assignee | ||
Comment 3•5 years ago
|
||
Yes, I expanded the scope a bit, as in reality the UI shouldn't allow the cut_action to be generated in the first place. Would a more-targeted solution/test of the new code such as the attached be sufficient for now? I can follow up with a bug to address the more-unlikely case where a cut_action is generated by something like an add-on, if you feel there's value in protecting there. I'm still scoping how much additional attention we'll be paying into the SelectionHandler area with the new non-native selection logic sneaking up on us :-)
Attachment #8486160 -
Attachment is obsolete: true
Attachment #8488839 -
Flags: review?(wjohnston)
Updated•5 years ago
|
Attachment #8488839 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 4•5 years ago
|
||
TRY push https://tbpl.mozilla.org/?tree=Try&rev=9c38d0a7396a
Assignee | ||
Comment 5•5 years ago
|
||
On to fx-team https://hg.mozilla.org/integration/fx-team/rev/5ea18dc3e655
Comment 6•5 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5ea18dc3e655
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in
before you can comment on or make changes to this bug.
Description
•