645814 - Panel background is dark gray on OS X

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Will Bamberg [:wbamberg]]

Attachment: reddit-panel example

Try examples/reddit-panel: the panel appears as in the attached screenshot. This is platform-specific: it looks light gray on Ubuntu (but appears white in the main browser window on all platforms: http://www.reddit.com/.mobile?keep_extension=True).

Comment 1

[Alexandre Poirot [:ochameau]]

I think that arrow panels are responsive of that!
Regular panels don't have particular background color,
whereas arrow panels have a very specific one, that may be dark gray on Mac.

Comment 2

[Will Bamberg [:wbamberg]]

Thanks Alex. So if you supply your own HTML content for the panel, you can set the background color to be what you want. But if you are loading someone else's content, as in the reddit-panel example, what should you do?

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Remove panel background

Will: Can you tell me if this fix the problem ?
If not, can you change this patch and add a white background instead of a transparent one.

Comment 4

[Will Bamberg [:wbamberg]]

Yes, "white" works. The arrow itself is still gray, but I guess that's intentional.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

This fix breaks addons' ability to use the OS-default panel background color.  I'd rather we use a content script to explicitly set the background color for the reddit-panel example addon.

Comment 6

[David Illsley]

Hi, I just ran across the fact that between b3 and b4, the background colour for the panel changed from white to dark grey in my case (on OSX).

Please can you clarify... Is the default background colour not consistent across platforms? If not, how much does it vary by? i.e. Is it possible that I set a text colour of 'white' which contrasts well on the native panel on OSX and it disappears entirely on a while panel on a different platform?

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> Hi, I just ran across the fact that between b3 and b4, the background colour
> for the panel changed from white to dark grey in my case (on OSX).

Yes, that's right.  We updated panels in b4 to use Firefox's default style.


> Please can you clarify... Is the default background colour not consistent
> across platforms? If not, how much does it vary by? i.e. Is it possible that I
> set a text colour of 'white' which contrasts well on the native panel on OSX
> and it disappears entirely on a while panel on a different platform?

Yes, the default background color is not consistent across platforms.  It can vary quite significantly.  But you shouldn't have to set a text color yourself; the SDK should automatically choose a text color that contrasts well with the background color on each platform.

Comment 8

[Alexandre Poirot [:ochameau]]

(In reply to comment #7)
> (In reply to comment #6)
> > Hi, I just ran across the fact that between b3 and b4, the background colour
> > for the panel changed from white to dark grey in my case (on OSX).
> 
> Yes, that's right.  We updated panels in b4 to use Firefox's default style.
> 
> 
> > Please can you clarify... Is the default background colour not consistent
> > across platforms? If not, how much does it vary by? i.e. Is it possible that I
> > set a text colour of 'white' which contrasts well on the native panel on OSX
> > and it disappears entirely on a while panel on a different platform?
> 
> Yes, the default background color is not consistent across platforms.  It can
> vary quite significantly.  But you shouldn't have to set a text color yourself;
> the SDK should automatically choose a text color that contrasts well with the
> background color on each platform.

Myk: That's not the case as text is in an iframe, so by default the text color is black and panel text style is not applied across iframe document, right ?

Comment 9

[David Illsley]

Attachment: Simple Panel on OSX without any background colour set

Myk, that's certainly not what I'm seeing at the moment (see attached image). Should I raise a separate bug for that?

Comment 10

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #8)
> (In reply to comment #7)
> > But you shouldn't have to set a text color yourself;
> > the SDK should automatically choose a text color that contrasts well with the
> > background color on each platform.
> 
> Myk: That's not the case as text is in an iframe, so by default the text color
> is black and panel text style is not applied across iframe document, right ?

Right, that's what currently happens.  But we should apply the text color (and other rules, as appropriate) to the iframe so it looks like an attractive, platform-native panel by default.


(In reply to comment #9)
> Myk, that's certainly not what I'm seeing at the moment (see attached image).
> Should I raise a separate bug for that?

Yes, please!

Comment 11

[Alexandre Poirot [:ochameau]]

As I don't think we can write CSS rules that apply accross document,
we need to add some CSS rules into the panel iframe document.
We should wait for having platform capabilities I was talking about in bug 653495, comment 6 in order to do so.
And even if we have such capabilities, I don't think it's a good idea to store style definition in the SDK ? As it will break themes and be hard to maintain.
We may get some computed style values and apply them in the iframe ?

Do you see anything simplier that can be done with current platform capabilities without messing with theming?

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Fix only reddit example by setting an explicit background in its content script

Here is a first way to "fix" this bug.
Only fix this particular example manually.

Comment 13

[Alexandre Poirot [:ochameau]]

Attachment: Fix all panels background comings from http(s) by setting white background

Here is another way. I'm more into such solution as websites are designed to work on white background. 
Loading remote document in a panel is very different from loading a local document from data folder. If we consider loading any website modded through content script, it may be relevant to set a white background by default to the iframe for these remote documents.

Comment 14

[Alexandre Poirot [:ochameau]]

David opened bug 652548 and you may take a look at proposed patch that is related to this bug.

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 535057 [details] [diff] [review]
Fix all panels background comings from http(s) by setting white background

I like this fix in theory, since it makes the common case just work, but it prevents addons from making the background transparent, which should remain possible.

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 535056 [details] [diff] [review]
Fix only reddit example by setting an explicit background in its content script

>diff --git a/examples/reddit-panel/data/panel.js b/examples/reddit-panel/data/panel.js

>+// As panel iframe is transparent by default, we need to explicitly define a 
>+// white background as MacOS panel background is dark gray.

Nit:

// Panels have an OS-specific background color by default, and the Mac OS X
// background color is dark grey, but Reddit expects its background to be white
// and looks odd when it isn't, so set it to white.

r=myk

Comment 17

[Alexandre Poirot [:ochameau]]

Landed:
https://github.com/mozilla/addon-sdk/commit/6d891145e83e0fa0fb1d91e4db4b524b1a7ee8de