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

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

({verified1.9.1})

unspecified
All
macOS
Points:
---
Bug Flags:
blocking-firefox3.5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [geo])

Attachments

(3 attachments, 1 obsolete attachment)

Posted image 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.
on the mac, would it be okay to use the same color as the text to the left of the link?
Summary: Blue "Learn More..." link in geopromp is hard to read → Blue "Learn More..." link in geolocation notification bar is hard to read
Whiteboard: [geo]
Duplicate of this bug: 489484
Attachment #374118 - Flags: ui-review?(beltzner)
Posted patch patch v.1 (obsolete) — Splinter Review
Attachment #374119 - Flags: review?
Attachment #374119 - Flags: review? → review?(gavin.sharp)
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.
(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.
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.
(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.
notification[type="info"] .text-link !important {
  color: rgba(255,255,255,0.95);
}

in browser.css?
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 on attachment 374119 [details] [diff] [review]
patch v.1

(cancelling request since comment 5 - comment 10 needs to be addressed)
Attachment #374119 - Flags: review?(gavin.sharp)
Posted patch p v.2Splinter Review
Attachment #374119 - Attachment is obsolete: true
Attachment #375258 - Flags: review?(gavin.sharp)
Attachment #375258 - Flags: review?(gavin.sharp) → review+
Attachment #375258 - Flags: approval1.9.1?
http://hg.mozilla.org/mozilla-central/rev/9b78b9dbdb0c
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: blocking-firefox3.5?
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Flags: blocking-firefox3.5? → blocking-firefox3.5+
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.
Attachment #375258 - Flags: approval1.9.1?
Comment on attachment 374118 [details]
screen shot w/ grey text

removing request. this bug has been fixed.
Attachment #374118 - Flags: ui-review?(beltzner)
You need to log in before you can comment on or make changes to this bug.