945251 - Reader mode toolbar tap highlight colour is orange instead of grey

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Reader View, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

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.

Comment 1

[:Margaret Leibovic]

Can we get a regression window for this? And a screenshot?

Comment 2

[Teodora Vermesan (:TeoVermesan)]

Attachment: Nightly_toolbar

Comment 3

[Teodora Vermesan (:TeoVermesan)]

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?

Comment 4

[Aaron Train [:aaronmt]]

I believe so.

Comment 5

[Wesley Johnston (:wesj)]

Heh. Reader specifies this. It just started working finally :)

http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css#181

Comment 6

[Sofia Larsson (:karinsofia)]

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;
}

Comment 7

[Wesley Johnston (:wesj)]

Sorry for the delay. Yeah, I think that's all we need here though! Yay for easy fixes!

Comment 8

[Sofia Larsson (:karinsofia)]

Attachment: Removing the orange color for active buttons

Thanks! Here comes a patch with the removal. :)

Comment 9

[Wesley Johnston (:wesj)]

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 <[e-mail]>"), 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 :)

Comment 10

[Wesley Johnston (:wesj)]

Oh this patch was good! Sorry bout that!

https://hg.mozilla.org/integration/fx-team/rev/14b3be07836d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/14b3be07836d

Comment 12

[Ioana Chiorean]

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.

Comment 13

[u421692]

Will this be uplifted to FF28?, so we can change the bug status to VERIFIED.

Comment 14

[Wesley Johnston (:wesj)]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d5c4a49d8d6d

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/d5c4a49d8d6d

Comment 17

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 28 Beta 6
Device: LG Nexus 4
OS: Android 4.4