Closed Bug 855352 Opened 7 years ago Closed 2 years ago

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

Categories

(Core :: Layout: Form Controls, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: ahal, Unassigned)

References

Details

(Whiteboard: [leave open])

Attachments

(2 files, 1 obsolete file)

Vivien, is there any stylesheet rules that could apply to <input type='file'> on B2G specifically?
Flags: needinfo?(21)
(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
Flags: needinfo?(21)
Duplicate of this bug: 856186
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.
Can we get someone to take a look at these? This is still one of the bugs blocking reftests from being unhidden.
Jonas, is there anyone in the B2G team who could work on that?
Flags: needinfo?(jonas)
Because we left this reftest hunk hidden because of this permaorange, we've now landed b2g bustage in other tests in the same hunk.
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
Whiteboard: [leave open]
Assignee: nobody → jwwang
Attached patch style patch (obsolete) — Splinter Review
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?
patch that fixes
== input-range-moz-range-progress-1.html input-range-moz-range-progress-1-ref.html
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.
Attachment #747863 - Flags: review?(mounir)
Flags: needinfo?(jonas)
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.
Attachment #747863 - Flags: review?(mounir) → review-
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.
Flags: needinfo?(21)
(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.
Flags: needinfo?(21)
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.
Attachment #747863 - Attachment is obsolete: true
(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.
Depends on: 871960
Mounir: Could you clarify what you think the way forward is here?
(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.
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.
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.
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?
(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?
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)
(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?
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...
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.
Release the bug so someone can take it.
Assignee: jwwang → nobody
B2G is no more.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.