855352 - input-file-simple.html, input-file-rtl.html, input-file-size.html fail on B2G emulator reftest due to differing font colour

Status: RESOLVED INVALID

(Core :: Layout: Form Controls, defect)

Description

[Andrew Halberstadt [:ahal]]

See: https://tbpl.mozilla.org/php/getParsedLog.php?id=21163766&tree=Mozilla-Inbound&full=1

These tests were introduced in bug 839787.

Comment 1

[Mounir Lamouri (:mounir)]

Vivien, is there any stylesheet rules that could apply to <input type='file'> on B2G specifically?

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Mounir Lamouri (:mounir) from comment #1)
> Vivien, is there any stylesheet rules that could apply to <input
> type='file'> on B2G specifically?

You may want to look at b2g/chrome/content/content.css

Comment 4

[Mounir Lamouri (:mounir)]

The simplest way to fix this would be to revert the b2g specific styles (from b2g/chrome/content/content.css) in the tested html files in the reftests. We could have different test reference for b2g vs non-b2g but I do not believe it is worth the hassle.

Comment 5

[Andrew Halberstadt [:ahal]]

Can we get someone to take a look at these? This is still one of the bugs blocking reftests from being unhidden.

Comment 6

[Mounir Lamouri (:mounir)]

Jonas, is there anyone in the B2G team who could work on that?

Comment 7

[Phil Ringnalda (:philor)]

Because we left this reftest hunk hidden because of this permaorange, we've now landed b2g bustage in other tests in the same hunk.

Comment 8

[Phil Ringnalda (:philor)]

Though as it turns out, we only landed the extra bustage 12 hours ago.

Annotated the failures so I can unhide it before the next bustage, in https://hg.mozilla.org/integration/mozilla-inbound/rev/0921a25af05d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0921a25af05d

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Attachment: style patch

This patch could fix the first 3 fails.
== input-file-simple.html input-file-simple-ref.xul
== input-file-rtl.html input-file-rtl-ref.xul
== input-file-size.html input-file-simple-ref.xul

However, it might affect other test cases which include this style file. It might be more prudent to override the style on per file base to avoid side effects, thought?

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

Attachment: input-range-moz-range-progress-1.html.patch

patch that fixes
== input-range-moz-range-progress-1.html input-range-moz-range-progress-1-ref.html

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 747863 [details] [diff] [review]
style patch

Review of attachment 747863 [details] [diff] [review]:
-----------------------------------------------------------------

Since these are Mounir's tests, I think it's his call.

Comment 13

[Mounir Lamouri (:mounir)]

Comment on attachment 747863 [details] [diff] [review]
style patch

Review of attachment 747863 [details] [diff] [review]:
-----------------------------------------------------------------

That patch might be green on B2G but would fail on all other platforms.

Comment 14

[Mounir Lamouri (:mounir)]

Is there a reason why Firefox OS has hard coded colors instead of changing -moz-FieldText (and other system color names)? I wonder if the solution wouldn't be to fix this.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Mounir Lamouri (:mounir) from comment #14)
> Is there a reason why Firefox OS has hard coded colors instead of changing
> -moz-FieldText (and other system color names)? I wonder if the solution
> wouldn't be to fix this.

There is no particular reasons for colors defined via color: versus anything else.

This file has been made with 3 things in mind: 
 - Have a pretty decent look and field for forms element for platform where we don't use the native theming mechanism.
 - Do not prevent websites to overidde the default theme
 - Do not regress reftests

As long as both of those goals are kept in mind you can change it the way you want.

Comment 16

[JW Wang [:jwwang] [:jw_wang]]

Attachment: style patch take 2

This patch adds style overrides in the html files so that this test is immune to changes in the global style which is platform-dependent as suggested by Mounir.

Comment 17

[Mounir Lamouri (:mounir)]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #15)
> (In reply to Mounir Lamouri (:mounir) from comment #14)
> > Is there a reason why Firefox OS has hard coded colors instead of changing
> > -moz-FieldText (and other system color names)? I wonder if the solution
> > wouldn't be to fix this.
> 
> There is no particular reasons for colors defined via color: versus anything
> else.
> 
> This file has been made with 3 things in mind: 
>  - Have a pretty decent look and field for forms element for platform where
> we don't use the native theming mechanism.
>  - Do not prevent websites to overidde the default theme
>  - Do not regress reftests
> 
> As long as both of those goals are kept in mind you can change it the way
> you want.

Those goals do not look very specific to Gaia/FxOS FWIW.

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Mounir: Could you clarify what you think the way forward is here?

Comment 19

[Mounir Lamouri (:mounir)]

(In reply to Jonas Sicking (:sicking) from comment #18)
> Mounir: Could you clarify what you think the way forward is here?

See bug 871960. The solution is to have -moz-textField and other system colours defined the way Firefox OS wants them because for the moment, content.css is using hardcoded colours which is bad for various reasons (see bug 871960). Trying to work around this problem in the test file isn't worth it. We should fix the cause of the problem and the test should likely pass.

Comment 20

[JW Wang [:jwwang] [:jw_wang]]

The purpose of reftest is to test a given visual effect can be achieved by more than one ways. Any factors that can result in different render results must be introduced in a controlled manner. Browser default styles are platform-dependent and less controllable. It takes effort to maintain the consistency between different browser default styles on different platforms so that reftests can pass.

The first 3 fails can be solved by an extra level of indirection where text referencing the color returned by -moz-textField and also allow text color customization on different platforms if -moz-textField return different colors on different platforms. However, fail#4 is another case. In the end of this approach, We may have to create many functions for different styles without hard-coding them.

I would rather get rid of browser default styles in the reftests and use only internal or inline styles for they are scoped and controllable if this is possible.

Comment 21

[Mounir Lamouri (:mounir)]

I disagree. This reftest is actually showing a bug: the <input type=file> widget in Firefox OS isn't using the default colours but some arbitrary ones. Gonk can define the value of those default colours, users can define those values, themes can define those values. It is very important to use those default colours instead of arbitrary ones. Those tests should pass if Firefox OS was using those default colours. We would indeed make the tests pass by setting another set of arbitrary colours in the tests but that would just hide the real cause of the initial failure.

Comment 22

[JW Wang [:jwwang] [:jw_wang]]

How about test#4(input-range-moz-range-progress-1.html)?

Should we also have default background, default border-style, and default border-radius applying to both <input> and <div> for test#4 to pass?

Comment 23

[Mounir Lamouri (:mounir)]

(In reply to jwwang from comment #22)
> How about test#4(input-range-moz-range-progress-1.html)?
> 
> Should we also have default background, default border-style, and default
> border-radius applying to both <input> and <div> for test#4 to pass?

This is out of the scope of that bug. Could you open a different bug for that?

Comment 24

[JW Wang [:jwwang] [:jw_wang]]

Btw, changing the color in b2g/chrome/content/content.css doesn't seem to affect the result of input-file-simple.html. Is the style file actually applied?

(running reftest on emulator with the command: python runreftestb2g.py --b2gpath $B2G_HOME --xre-path /home/jwwang/tools/xre --emulator arm --emulator-res 800x1000 --ignore-window-size tests/layout/reftests/forms/input/file/reftest.list)

Comment 25

[Mounir Lamouri (:mounir)]

(In reply to jwwang from comment #24)
> Btw, changing the color in b2g/chrome/content/content.css doesn't seem to
> affect the result of input-file-simple.html. Is the style file actually
> applied?

Did you remove the color overriding in content.css?

Comment 26

[JW Wang [:jwwang] [:jw_wang]]

yes, I removed all lines from content.css except the following:

@namespace url("http://www.w3.org/1999/xhtml");
@namespace xul url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");

input {
    color: #FFFF00;
}

The text color is supposed to be yellow but wasn't...

Comment 27

[JW Wang [:jwwang] [:jw_wang]]

Never mind. It looks like you have to build the whole b2g codebase and re-run the reftest. Just modifying content.css and running reftest again won't take effect. I wonder if the content.css is packed into some resource file which is read by the emulator.

Comment 28

[JW Wang [:jwwang] [:jw_wang]]

Release the bug so someone can take it.

Comment 29

[Anne (:annevk)]

B2G is no more.