Closed Bug 1247796 Opened 4 years ago Closed 4 years ago

Element with background-color:ActiveBorder renders all black

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: ashughes, Assigned: miketaylr)

Details

Attachments

(2 files, 1 obsolete file)

Attached file test.html
I first noticed this issue on Fujifilm's repair status page:
http://repair.fujifilm.ca/aysstatus/result.php

When I looked up the information for my camera all I saw was a blacked out div, however I could see the text if I highlighted everything on the page. In Chrome the page displayed correctly.

Upon inspection I noticed that the container div was using background-color:ActiveBorder and that disabling this style made the text appear. I've attached a minimized testcase which shows the behaviour.

I'm not sure if this is expected but Firefox seems to be the only browser that behaves this way.
Looks like this behaviour goes back at least as far as Firefox 24 so probably not a regression.
I'm not really sure what our compatibility requirements are for system colours.  FTR, what ActiveBorder maps to depends on the platform.  On OS X we map it to black (presumably because the system doesn't have something we can ask for):

  https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsLookAndFeel.mm#171

though Chrome and Safari on OS X both use a blue (looks like the selection colour).

On Windows we ask for the theme's active window border color:

  https://dxr.mozilla.org/mozilla-central/source/widget/windows/nsLookAndFeel.cpp#148

which on Windows 10 with the default theme gives me a medium grey.  This is the same in Edge, but Chrome on Windows uses white.
Using ActiveBorder as a background design element on a webpage seems like an interesting choice... given that it's all over the place between OSes and browsers.

But, our choice of black seems to be the most likely to cause a problem if people are using it behind text (with a default color of black). Aligning with Chrome, Safari and Opera here probably isn't a bad idea on the web. I suppose the risk in doing so is changing people's expectations for Mozilla-only code.

(This does feel very edge-casey, so we could also try to just ask Fuji to fix their site if someone feels strongly against changing this).
(In reply to Cameron McCormack (:heycam) (away Feb 27 – Mar 13) from comment #2)
> I'm not really sure what our compatibility requirements are for system
> colours.  FTR, what ActiveBorder maps to depends on the platform.  On OS X
> we map it to black (presumably because the system doesn't have something we
> can ask for):

Chrome and Safari map to [NSColor keyboardFocusIndicatorColor], so let's do the same (I'm guessing that didn't exist years ago when black was picked).

According to https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSColor_Class/#//apple_ref/occ/clm/NSColor/keyboardFocusIndicatorColor, it was added in OSX v10 -- looks safe to use.
Assignee: nobody → miket
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5406286f8cb

(I guess there aren't any reftests to update here, since this is OS-specific)
This matches what Safari and Chrome do on Mac.
Comment on attachment 8724090 [details] [diff] [review]
Use keyboardFocusIndicatorColor for ActiveBorder system color keyword. r=?

bz, Bugzilla suggests you for reviewing. Feel free to redirect as appropriate, thanks!
Attachment #8724090 - Flags: review?(bzbarsky)
Comment on attachment 8724090 [details] [diff] [review]
Use keyboardFocusIndicatorColor for ActiveBorder system color keyword. r=?

Bugzilla seems moderately on crack.  Let's try someone who knows a bit more about this file... ;)
Attachment #8724090 - Flags: review?(bzbarsky) → review?(mstange)
Attachment #8724090 - Flags: review?(mstange) → review+
(Heh, thanks Boris -- I was impressed you're also an expert in OSX theming. ^_^)

Thanks for the review Markus!
This matches what Safari and Chrome do on Mac.
Attachment #8724258 - Flags: review?(mstange)
Attachment #8724090 - Attachment is obsolete: true
Comment on attachment 8724258 [details] [diff] [review]
Use keyboardFocusIndicatorColor for ActiveBorder system color keyword.

(Oops, sorry for double review request -- still learning git bz)

Updating commit message and carrying forward r+.
Attachment #8724258 - Flags: review?(mstange) → review+
Try run from Comment #5 is green, so let's push this.
https://hg.mozilla.org/mozilla-central/rev/968f6c420cf7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.