488884 - Blue "Learn More..." link in geolocation notification bar is hard to read

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Doug Turner (:dougt)]

Attachment: screen shot

The Blue colored link on the gray info bar background is hard to read.  We should change the color on the mac to be easier to read.

Comment 1

[Doug Turner (:dougt)]

on the mac, would it be okay to use the same color as the text to the left of the link?

Comment 3

[Doug Turner (:dougt)]

Attachment: screen shot w/ grey text

Comment 4

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 374119 [details] [diff] [review]
patch v.1

I think you only want this in the notification[type="info"] case, since the background is red or yellow in the other cases. Seems like we should also use exactly the same color. So:

notification[type="info"] .text-link {
  color: rgba(255,255,255,0.95);
}

Is the !important really needed?

The only other question is whether this belongs in a global toolkit rule or not (i.e. whether all notification bars with text-links should be forced to have this styling). I'm somewhat ambivalent, since it's probably a fairly rare case in practice. If the !important is required, that might negatively impact users who want some other styling, though, which is worth considering.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> The only other question is whether this belongs in a global toolkit rule or not
> (i.e. whether all notification bars with text-links should be forced to have
> this styling).

I'd say the answer to this is yes, as the default blue will never be the right choice for the background that notification.css sets for type="info" bars.
But I'd also add a comment about why the rule is there at all, since notification bars don't support text-links by default.

Comment 7

[Doug Turner (:dougt)]

gavin, should I just set this style in nsBrowserGlue since, as Dão points out, notification bars do not support text-links anyhow.

the !important was needed because the class already has a color associated with it and without the !important, that is was was being used.

Comment 8

[Dão Gottwald [:dao]]

(In reply to comment #7)
> gavin, should I just set this style in nsBrowserGlue

The notification background color is theme-dependent, so that won't work. The link color should be set in notification.css or browser.css.

Comment 9

[Doug Turner (:dougt)]

notification[type="info"] .text-link !important {
  color: rgba(255,255,255,0.95);
}

in browser.css?

Comment 10

[Dão Gottwald [:dao]]

If you want it in browser.css, you should use a geolocation-specific class name ("geolocation-learn-more-link" or something like that), otherwise you might as well put it in notification.css.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 374119 [details] [diff] [review]
patch v.1

(cancelling request since comment 5 - comment 10 needs to be addressed)

Comment 12

[Doug Turner (:dougt)]

Attachment: p v.2

Comment 13

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/9b78b9dbdb0c

Comment 14

[Marcia Knous [:marcia]]

Verified fixed using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090501 Minefield/3.6a1pre. Very easy to see when visiting http://outside.in/radar/welcome.

Comment 15

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8eb9de885e58

Comment 16

[Marcia Knous [:marcia]]

Verified fixed on the 1.9.1 branch using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090505 Shiretoko/3.5b5pre. Adding keyword.

Comment 17

[Doug Turner (:dougt)]

Comment on attachment 374118 [details]
screen shot w/ grey text

removing request. this bug has been fixed.