579208 - Sizes of some XUL elements changes when lightweight themes are applied

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[[e-mail]]

Unlike Firefox, we don't override the borders of our toolbars and menus. So when lightweight themeing turns them off, their size changes, which is ugly and hopefully unintentional. The size of statusbarpanels also changes, but I haven't investigated them yet.

Comment 1

[[e-mail]]

Attachment: Possible patch

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 457704 [details] [diff] [review]
Possible patch

I don't think we want the border visually, so this should probably use border-color: transparent instead...

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 457704 [details] [diff] [review]
Possible patch

Also, how will this affect cases where the border wasn't actually used without the lightweight theme due to -moz-appearance?

Comment 4

[[e-mail]]

(In reply to comment #2)
> I don't think we want the border visually, so this should probably use
> border-color: transparent instead...
Sure, I can switch winstripe to border-color: transparent if you prefer.

(In reply to comment #3)
> Also, how will this affect cases where the border wasn't actually used without
> the lightweight theme due to -moz-appearance?
Well, only winstripe provides a CSS border, so if the other themes still change size then I guess they'll need to add a CSS border width.

Comment 5

[[e-mail]]

Attachment: Screen shot with borders

Personally I don't think there's anything wrong with keeping the borders. At least this way the application can turn them off it doesn't want them.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #4)
> (In reply to comment #3)
> > Also, how will this affect cases where the border wasn't actually used without
> > the lightweight theme due to -moz-appearance?
> Well, only winstripe provides a CSS border, so if the other themes still change
> size then I guess they'll need to add a CSS border width.

This doesn't seem to answer my question. Winstripe specifies a CSS border, but that border isn't used in case of -moz-appearance:toolbar, so it looks like your patch will increase the toolbar height rather than maintaining it.

Comment 7

[[e-mail]]

(In reply to comment #6)
> This doesn't seem to answer my question. Winstripe specifies a CSS border, but
> that border isn't used in case of -moz-appearance:toolbar, so it looks like
> your patch will increase the toolbar height rather than maintaining it.
Which OS is this on? My toolbars on XP have borders with Luna or Classic.

Comment 8

[Dão Gottwald [:dao]]

I'm not sure about various third-party OS themes in general. Luna draws borders, but do these actually occupy space like the CSS borders would?

Comment 9

[[e-mail]]

Attachment: Proposed patch

OK, so comparing with gnomestripe we should make the borders transparent.

Tested on both Classic and Luna.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 460328 [details] [diff] [review]
Proposed patch

It's still not fully clear to me that this will do the right thing when switching from non-lwtheme (-moz-appearance:toolbar, no CSS border rendered) to lwtheme (-moz-appearance:none, transparent CSS border rendered), but rs=me

Comment 11

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/be49c680d2f5

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/be49c680d2f5

Comment 13

[Guilherme Lima]

This patch caused 2 bugs:
1 - on a maximized window, there is now a space above the tabs - so just moving the cursor to the top of the window and clicking won't select a tab, which is a bit annoying... having to target the tab.
2 - there is a white line between the current tab and the rest of the window.
This bugs happen when using a lw-theme. Normal theme has no problems.

Comment 14

[Dão Gottwald [:dao]]

(In reply to Guilherme Lima from comment #13)
> This patch caused 2 bugs:
> 1 - on a maximized window, there is now a space above the tabs - so just
> moving the cursor to the top of the window and clicking won't select a tab,
> which is a bit annoying... having to target the tab.
> 2 - there is a white line between the current tab and the rest of the window.
> This bugs happen when using a lw-theme. Normal theme has no problems.

Could you please file a new bug on this and make it block this one? Thanks.

Comment 15

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #14)
> (In reply to Guilherme Lima from comment #13)
> > This patch caused 2 bugs:
> > 1 - on a maximized window, there is now a space above the tabs - so just
> > moving the cursor to the top of the window and clicking won't select a tab,
> > which is a bit annoying... having to target the tab.
> > 2 - there is a white line between the current tab and the rest of the window.
> > This bugs happen when using a lw-theme. Normal theme has no problems.
> 
> Could you please file a new bug on this and make it block this one? Thanks.

filed bug 1101495