Closed Bug 645814 Opened 11 years ago Closed 11 years ago

Panel background is dark gray on OS X

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wbamberg, Assigned: ochameau)

Details

(Whiteboard: [papercut])

Attachments

(5 files)

Attached image 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).
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.
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?
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.
Yes, "white" works. The arrow itself is still gray, but I guess that's intentional.
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.
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Whiteboard: [papercut]
Target Milestone: --- → 1.0
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?
(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.
(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 ?
Myk, that's certainly not what I'm seeing at the moment (see attached image). Should I raise a separate bug for that?
(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!
Assignee: nobody → myk
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?
Here is a first way to "fix" this bug.
Only fix this particular example manually.
Assignee: myk → poirot.alex
Attachment #535056 - Flags: review?(myk)
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.
Attachment #535057 - Flags: review?(myk)
David opened bug 652548 and you may take a look at proposed patch that is related to this bug.
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.
Attachment #535057 - Flags: review?(myk) → review-
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
Attachment #535056 - Flags: review?(myk) → review+
Landed:
https://github.com/mozilla/addon-sdk/commit/6d891145e83e0fa0fb1d91e4db4b524b1a7ee8de
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.