Closed Bug 1174755 Opened 9 years ago Closed 9 years ago

Reftest failures on GTK3 due to native theme background

Categories

(Core :: Widget: Gtk, defect)

Unspecified
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: acomminos, Assigned: acomminos)

References

Details

Attachments

(1 file)

Some reftests fail with the GTK3 theme used on the try server due to native theming of some XUL/HTML elements. The tests should be modified to pass regardless of the native theme's background.

Example: layout/reftests/bugs/664127-1.xul
This patch makes several layout reftests independent of the platform's widget styling.

It also bumps up the fuzziness for a bidi test already fuzzed on GTK3.
Attachment #8622554 - Flags: review?(karlt)
Comment on attachment 8622554 [details] [diff] [review]
Improve reftest consistency on GTK3.

Better to ask the author of the tests to review these changes.
Mats authored at least some of them.
Current rendering can be seen on
https://treeherder.mozilla.org/#/jobs?repo=elm
Attachment #8622554 - Flags: review?(karlt) → review?(mats)
Comment on attachment 8622554 [details] [diff] [review]
Improve reftest consistency on GTK3.

(In reply to Andrew Comminos [:acomminos] from comment #0)
> Some reftests fail with the GTK3 theme used on the try server due to native
> theming of some XUL/HTML elements. The tests should be modified to pass
> regardless of the native theme's background.

When an element is styled with "-moz-appearance:none" then the native theme's
background, or anything else from native theme, shouldn't applied in the first
place.  These test failures seems to indicate a bug in that regard.

The change to 966992-1.html / 966992-1-ref.html seems to indicate a second bug,
separate from the one above.  The changes in the patch for these two files
makes no sense to me.  Can you explain what the error in the reference
rendering comes from please?
Flags: needinfo?(karlt)
Flags: needinfo?(acomminos)
Attachment #8622554 - Flags: review?(mats) → review-
(In reply to Mats Palmgren (:mats) from comment #4)
> Comment on attachment 8622554 [details] [diff] [review]
> Improve reftest consistency on GTK3.
> 
> (In reply to Andrew Comminos [:acomminos] from comment #0)
> > Some reftests fail with the GTK3 theme used on the try server due to native
> > theming of some XUL/HTML elements. The tests should be modified to pass
> > regardless of the native theme's background.
> 
> When an element is styled with "-moz-appearance:none" then the native theme's
> background, or anything else from native theme, shouldn't applied in the
> first
> place.  These test failures seems to indicate a bug in that regard.
> 
> The change to 966992-1.html / 966992-1-ref.html seems to indicate a second
> bug,
> separate from the one above.  The changes in the patch for these two files
> makes no sense to me.  Can you explain what the error in the reference
> rendering comes from please?

Reftest analyzer results showing differences: http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/elm-linux-debug/1434698441/elm_ubuntu32_vm-debug_test-reftest-2-bm02-tests1-linux32-build1.txt.gz&only_show_unexpected=1

The tests for bugs 664127 and 668319 attempt to verify that setting the opacity of the tree parent properly sets the opacity of the children- this fails on themes that set a nonwhite background colour for the tree.

For bug 966992, the test assumes that the text box's background colour is solid white, as evidenced by the application of a white mask.

I agree, however, that making moz-appearance ensure that no native styling is introduced would be the best solution. I'd imagine it could break quite a few tests, perhaps we should handle it in a separate bug?
Flags: needinfo?(acomminos) → needinfo?(mats)
(In reply to Andrew Comminos [:acomminos] from comment #5)
> For bug 966992, the test assumes that the text box's background colour is
> solid white, as evidenced by the application of a white mask.

Ah, now I see where that block comes from.  Yes, this test assumes that
-moz-appearance:none results in a solid white background, which is a valid
assumption.

> I agree, however, that making moz-appearance ensure that no native styling
> is introduced would be the best solution. I'd imagine it could break quite a
> few tests, perhaps we should handle it in a separate bug?

Yes, that's a bug that should be fixed.  I bet it will fix the test
failures in this bug.
Flags: needinfo?(mats)
(In reply to Mats Palmgren (:mats) from comment #6)
> (In reply to Andrew Comminos [:acomminos] from comment #5)
> > For bug 966992, the test assumes that the text box's background colour is
> > solid white, as evidenced by the application of a white mask.
> 
> Ah, now I see where that block comes from.  Yes, this test assumes that
> -moz-appearance:none results in a solid white background, which is a valid
> assumption.

Why do you think this is a valid assumption?

> > I agree, however, that making moz-appearance ensure that no native styling
> > is introduced would be the best solution. I'd imagine it could break quite a
> > few tests, perhaps we should handle it in a separate bug?
> 
> Yes, that's a bug that should be fixed.  I bet it will fix the test
> failures in this bug.

Are you saying -moz-appearance: none should influence the used value of the background-color property? That doesn't sound right to me. What's the solution you're thinking of?
Flags: needinfo?(mats)
(In reply to Markus Stange [:mstange] from comment #7)
> Why do you think this is a valid assumption?

-moz-appearance:none disables native theming, that's mostly what it's for.
Thus we will render the elements according to the CSS styles instead.
The default UA style sheet for <input> has a solid white background.
As far as I know that has been the default for all platforms since
forever.  If this is no longer true in GTK3 builds then I consider
that difference a bug.

To be clear, I'm not objecting to changing the default built-in
styles for -moz-appearance:none form controls (bug 997190).
I'm objecting that it is *different* in GTK3 builds.  I strongly
believe -moz-appearance:none should have the same rendering on all
platforms.

> Are you saying -moz-appearance: none should influence the used value of the
> background-color property?

No.  -moz-appearance doesn't change the value of any (other)
CSS properties.

> What's the solution you're thinking of?

I don't know where they grey-ish background color comes from in
the GTK3 builds.  I'm guessing it comes from the native theme,
which is a bug since it shouldn't be used for -moz-appearance:none
elements.  If it comes from some other source, say some special
UA styles just for GTK3 builds, then I consider that a bug too
since default UA styles for form controls (that are used for
-moz-appearance:none) should be the same on all platforms.
Flags: needinfo?(mats)
(In reply to Mats Palmgren (:mats) from comment #8)
> I don't know where they grey-ish background color comes from in
> the GTK3 builds.  I'm guessing it comes from the native theme,
> which is a bug since it shouldn't be used for -moz-appearance:none
> elements.

IMHO that's incarnation of Bug 1158076 for the Light themes, see:

https://bugzilla.mozilla.org/show_bug.cgi?id=1158076#c15
(In reply to Mats Palmgren (:mats) from comment #8)
> The default UA style sheet for <input> has a solid white background.

Oh, so this is where our misunderstanding came from. I thought we were talking about the XUL <tree>'s background, which has background-color: -moz-Field in its UA stylesheet. I didn't catch that the patch contained style changes for <input> elements as well.

Andrew, do you know where the bad background color for input elements is coming from?
(In reply to Markus Stange [:mstange] from comment #10)
> Andrew, do you know where the bad background color for input elements is
> coming from?

The background colour for input elements is also set to -moz-Field according to forms.css- I'd suspect that's where it would be coming from.
(In reply to Andrew Comminos [:acomminos] from comment #11)
> The background colour for input elements is also set to -moz-Field according
> to forms.css- I'd suspect that's where it would be coming from.

btw. forms.css is used for HTML elements and only when it's not styled by OS. General XUL elements are defined in toolkit dir for each OS.
Comment on attachment 8622554 [details] [diff] [review]
Improve reftest consistency on GTK3.

Oh, so we side load values from the theme via these -moz-Field values?
I thought -moz-appearance:none neutralized those somehow, but I guess
it doesn't.

In light of that, this seems like a reasonable workaround.
Attachment #8622554 - Flags: review- → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1959e594ac43
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(In reply to Mats Palmgren (:mats) from comment #4)
> When an element is styled with "-moz-appearance:none" then the native theme's
> background, or anything else from native theme, shouldn't applied in the
> first
> place.  These test failures seems to indicate a bug in that regard.

(In reply to Mats Palmgren (:mats) from comment #8)
> -moz-appearance:none disables native theming, that's mostly what it's for.
> Thus we will render the elements according to the CSS styles instead.

> I strongly
> believe -moz-appearance:none should have the same rendering on all
> platforms.

I agree.

(In reply to Martin Stránský from comment #9)
> IMHO that's incarnation of Bug 1158076 for the Light themes

Yes.

(In reply to Mats Palmgren (:mats) from comment #13)
> Oh, so we side load values from the theme via these -moz-Field values?
> I thought -moz-appearance:none neutralized those somehow, but I guess
> it doesn't.

Setting CSS foreground and background colors from the theme causes problems
when mixed with document styling.

Avoiding setting CSS colors when -moz-appearance is none, or avoiding
altogether with -moz-appearance not-none causing foreground and background to
be obtained from the theme instead of CSS could resolve this.
Flags: needinfo?(karlt)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: