Content text selection highlight should match Gingerbread theming style

VERIFIED FIXED in Firefox 7

Status

Fennec Graveyard
General
--
enhancement
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: aaronmt, Assigned: mfinkle)

Tracking

({polish, verified-aurora})

Trunk
Firefox 7
ARM
Android
polish, verified-aurora
Dependency tree / graph

Details

(Whiteboard: [testday-2011-06-24])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 541802 [details]
Stock browser content text selection - white background

Mozilla/5.0 (Android; Linux armv7l; rv:7.0a1) Gecko/20110624 Firefox/7.0a1 Fennec/7.0a1

Currently content text selection is selected with a black background alongside grey text. Text that is highlighted becomes difficult to read. This should match the gingerbread style (stock browser)

For white backgrounds stock browser uses
Highlight selection: #FED27E

Black backgrounds stock browser uses
Highlight selection: #805400
(Reporter)

Comment 1

6 years ago
Created attachment 541804 [details]
Stock browser content text selection - black background
(Reporter)

Updated

6 years ago
Whiteboard: [testday-20110624]
(Reporter)

Updated

6 years ago
Blocks: 661388
Severity: normal → enhancement
Whiteboard: [testday-20110624] → [testday-2011-06-24]
Created attachment 545487 [details] [diff] [review]
patch plus more

This patch adds the CSS change and a few small fixes:
* -moz-selection in web content so we use the orange color in web content.
* Adds protection for the error in bug 667097. We need to avoid touching the selection object in the "end" message, so we cache the selected text during the "start" and "move" messages.
* Adds basic protection to avoid messing up the focus and selection if you move the handles over a textbox. This is fixed by the platform patch that allows us to not use fake clicks (bug 669816).
Assignee: nobody → mark.finkle
Attachment #545487 - Flags: review?(wjohnston)
Comment on attachment 545487 [details] [diff] [review]
patch plus more

Review of attachment 545487 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine and works fine here.

::: mobile/chrome/content/content.js
@@ +1407,5 @@
>  
>        case "Browser:SelectionMove":
> +        // Hack to avoid setting focus in a textbox [Bugs 654352 & 667243]
> +        let elemUnder = elementFromPoint(json.x - scrollOffset.x, json.y - scrollOffset.y);
> +        if (elemUnder && elemUnder instanceof Ci.nsIDOMHTMLInputElement || elemUnder instanceof Ci.nsIDOMHTMLTextAreaElement)

I half wonder if we need to combine this into a Utils function since we seem to check this sort of stuff other places:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/content.js#849
Attachment #545487 - Flags: review?(wjohnston) → review+
(In reply to comment #3)

> >        case "Browser:SelectionMove":
> > +        // Hack to avoid setting focus in a textbox [Bugs 654352 & 667243]
> > +        let elemUnder = elementFromPoint(json.x - scrollOffset.x, json.y - scrollOffset.y);
> > +        if (elemUnder && elemUnder instanceof Ci.nsIDOMHTMLInputElement || elemUnder instanceof Ci.nsIDOMHTMLTextAreaElement)
> 
> I half wonder if we need to combine this into a Utils function since we seem
> to check this sort of stuff other places:
> 
> http://mxr.mozilla.org/mobile-browser/source/chrome/content/content.js#849

If it was a long term fix, I would support moving to a util method, but we want to remove this if possible.
http://hg.mozilla.org/integration/mozilla-inbound/rev/225932c41c03
Whiteboard: [testday-2011-06-24] → [testday-2011-06-24] [inbound]
http://hg.mozilla.org/mozilla-central/rev/225932c41c03
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [testday-2011-06-24] [inbound] → [testday-2011-06-24]
Target Milestone: --- → Firefox 8
Mozilla/5.0 (Android; Linux armv71; rv8.0a1) Gecko/20110715 Firefox/8.0a1 Fennec/8.0a1
Device: HTC Flyer
OS: Android OS 2.3
Status: RESOLVED → VERIFIED
Comment on attachment 545487 [details] [diff] [review]
patch plus more

mobile only and it helps stabilize the text selection feature
Attachment #545487 - Flags: approval-mozilla-aurora?

Comment 9

6 years ago
Comment on attachment 545487 [details] [diff] [review]
patch plus more

Approved for mozilla-aurora
Attachment #545487 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/c6be7aeb4ef9
status-firefox7: --- → fixed
status-firefox8: --- → fixed
Target Milestone: Firefox 8 → Firefox 7
(Reporter)

Comment 11

6 years ago
Verified Fixed on Aurora
Mozilla/5.0 (Android; Linux armv7l; rv:7.0a2) Gecko/20110805 Firefox/7.0a2 Fennec/7.0a2
Keywords: verified-aurora
You need to log in before you can comment on or make changes to this bug.