754585 - Fennec Nightly failing ACID3 test (with one error)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Tobias (:Tobbi) Markus]

When running the acid3 test with nightly, I get 99/100 with the following error:
Test 47 failed: expected 'auto' but got 'none' - control failed.

Additionally, you see "You should not see this at all" in red font in the test result.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> Test 47 failed: expected 'auto' but got 'none' - control failed.

This isn't actually part of the test.  It's part of the test prerequisites: for test 47 to make sense as it's currently written, the computed 'cursor' value on the body must be "auto" (the initial value) if there are no other styles set by the page.

Fennec has the following rule in its UA stylesheet:

45 html|*:-moz-system-metric(touch-enabled) {
46   cursor: none !important;
47 }

which of course means that test 47 is meaningless.  And the test harness treats the meaninglessness as a test failure.

Arguably, this is a bug in the test.  I've e-mailed hixie about that.

> Additionally, you see "You should not see this at all"

There's an existing bug on this, I think...

Comment 3

[Boris Zbarsky [:bzbarsky]]

hixie is claiming that the cursor:none rule is a bug, fwiw, and refusing to fix the test.  Mark, up to you to fight that battle if you want to....

Comment 4

[Robert Kaiser]

This has the danger of making people say that Firefox for Android is buggy and giving it bad props for that, esp. as we are also already worse than some other browsers when it comes to the html5test stuff people like to cite for "browser quality" these days. And yes, being measured in results of those benchmarks sucks at times, but it's what happens in reality quite a bit.
Fortunately, we have an edge in speed right now on Android, but we wouldn't want people to water it by reporting "missing modern web functionality".

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Boris - Thanks for pin-pointing the reason for the failure. I removed the override just to see what would happen. Fennec does score 100/100 _and_ does not show the cursor. My guess is that the OMTC and GL changes removed the need to hide the cursor. Patch coming up.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch removes the cursor.css UA stylesheet file and the code that loads it. We score 100/100 on acid 3 and I don't see a cursor when tapping around web pages.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce75fa1a043e

CC'ing Vivien in case this affects b2g as well

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 638149 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): old code in fennec
User impact if declined: none. the impact is purely for hitting 100% in a benchmark
Testing completed (on m-c, etc.): minimal
Risk to taking this patch (and alternatives if risky): the code that was removed seemed to have no effect anymore. it was only needed in XUL fennec back before shadow layers was implemented.
String or UUID changes made by this patch: none

Comment 10

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

(In reply to Mark Finkle (:mfinkle) from comment #7)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ce75fa1a043e
> 
> CC'ing Vivien in case this affects b2g as well

Thanks for the CC. It doesn't since we don't use cursor.css on B2G.

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 638149 [details] [diff] [review]
patch

[Triage Comment]
Looks like a very simple removal of unused code - approving for Aurora 15 and Beta 14(.0.1).

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6c50580e7ec1
https://hg.mozilla.org/releases/mozilla-beta/rev/55d1dc068bab

Comment 13

[Aaron Train [:aaronmt]]

100% across the board, verified fixed on m-c (07/10), m-a (07/10), m-b (14.0 Beta 12, Build #1)