Closed Bug 945251 Opened 6 years ago Closed 6 years ago

Reader mode toolbar tap highlight colour is orange instead of grey

Categories

(Firefox for Android :: Reader View, defect)

28 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 29
Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 --- verified
firefox29 --- verified
firefox30 --- verified
b2g-v1.3 --- fixed

People

(Reporter: TeoVermesan, Assigned: karinsofia)

References

Details

(Keywords: regression, reproducible, Whiteboard: [mentor=wesj][lang=css])

Attachments

(2 files)

Tested with:
Build: Firefox for Android 28.0a1 (2013-12-02)
Device: LG Nexus 4
OS:Android 4.2.2.

Steps to Reproduce:
1. Open news.google.com and go to any article on the page
2. Enter reader mode
3. Tap on any option from the toolbar menu

Expected result:
- The tap highlight colour is grey.

Actual Result:
- The tap highlight colour is orange.
Can we get a regression window for this? And a screenshot?
Attached image Nightly_toolbar
Regression window-wanted:

mc:
not affected build: 14-11-2013
affected build: 15-11-2013  
-pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7b014f0f3b03&tochange=b2fab608772f

Did Bug 936593 created this issue?
I believe so.
Heh. Reader specifies this. It just started working finally :)

http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css#181
Whiteboard: [mentor=wesj][lang=css]
Hi!

Should active buttons ever be orange in reading mode?
Otherwise it should be enough to remove the declaration overriding the color:

.button:active {
  background-color: #ff9500;
}
Flags: needinfo?(wjohnston)
Sorry for the delay. Yeah, I think that's all we need here though! Yay for easy fixes!
Flags: needinfo?(wjohnston)
Thanks! Here comes a patch with the removal. :)
Attachment #8346770 - Flags: review?(wjohnston)
Comment on attachment 8346770 [details] [diff] [review]
Removing the orange color for active buttons

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

Thanks! For checkin we'll actually need a slightly different patch. There are some instructions here:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

but you'll need to modify your .hgrc to include a username (or call hg qref -u "My name <myname@something.com>"), and add a commit message (hg qref -m "Bug 123456 - My commit message. r=wesj")

I've got some other stuff to land tonight, so if you don't get something up by then, I can do both those for you. Thanks :)
Attachment #8346770 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/14b3be07836d
Assignee: nobody → karinsofiapaulina
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Nightly 29.0a1 (2013-12-26)
Galaxy Tab (Android 4.0.3 )

Going to Reader mode and long tapping one of the toolbar element will highlight it in gray not in orange.

Settings this to VERIFIED FIXED.
Status: RESOLVED → VERIFIED
Status: VERIFIED → RESOLVED
Closed: 6 years ago6 years ago
Will this be uplifted to FF28?, so we can change the bug status to VERIFIED.
Comment on attachment 8346770 [details] [diff] [review]
Removing the orange color for active buttons

Sure!

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Reader mode
User impact if declined: Wrong color highlight
Testing completed (on m-c, etc.): Has been on 29 for awhile.
Risk to taking this patch (and alternatives if risky): Very low risk. Removing some invalid CSS.
String or IDL/UUID changes made by this patch: None.
Attachment #8346770 - Flags: approval-mozilla-beta?
Attachment #8346770 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified fixed on:
Build: Firefox for Android 28 Beta 6
Device: LG Nexus 4
OS: Android 4.4
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.