Closed Bug 1575969 Opened 7 years ago Closed 6 years ago

Debian 10 - forms/input/number/number-selected.html == forms/input/number/number-selected-ref.html | image comparison, max difference: 202, number of differing pixels: 773

Categories

(Core :: Layout: Form Controls, defect, P5)

Product:
Core
Component:
Layout: Form Controls
Type:
defect

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: intermittent-bug-filer, Assigned: egao)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

screenshot of some dialogs from Ubuntu 16.04
320.54 KB, image/png
Details

Filed by: egao [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=262986639&repo=try
Full log: https://queue.taskcluster.net/v1/task/Rn9b_FIOQ2eHgkJ_hfk7TA/runs/0/artifacts/public/logs/live_backing.log
Reftest URL: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/Rn9b_FIOQ2eHgkJ_hfk7TA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

Platform: debian 10
Suite: reftest
Chunk: 1

Context:
[task 2019-08-22T18:20:55.738Z] 18:20:55 INFO - REFTEST TEST-START | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected-ref.html
[task 2019-08-22T18:20:55.739Z] 18:20:55 INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected.html | 18 / 24 (75%)
[task 2019-08-22T18:20:55.823Z] 18:20:55 INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected-ref.html | 18 / 24 (75%)
[task 2019-08-22T18:20:55.908Z] 18:20:55 INFO - REFTEST INFO | REFTEST fuzzy test (0, 0) <= (202, 773) <= (1, 1)
[task 2019-08-22T18:20:56.006Z] 18:20:56 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/forms/input/number/number-selected-ref.html | image comparison, max difference: 202, number of differing pixels: 773
[task 2019-08-22T18:20:56.006Z] 18:20:56 INFO - REFTEST IMAGE 1 (TEST):

:jwatt - this appears to be a new failure that is observed on new debian 10 linux test image. For more information on the migration to debian 10 test image, please see https://bugzilla.mozilla.org/show_bug.cgi?id=1565332.

I saw that you created the file back in 2013. Would you be able to take a look, or redirect the ni to someone that may be able to do so? Would an adjustment of the expected outcome be in order?

Flags: needinfo?(jwatt)

The number input field appears to be missing padding that is present in the reference. I'd suggest just annotating this for now.

Flags: needinfo?(jwatt)

I will also include annotation for number-style-inheritance.html which is also in the same reftest.list file.

Assignee: nobody → egao
Keywords: leave-open
See Also: → 1577072
Depends on: 1577256

This failure is just an indication that data:text/html,<input type="number"> is shorter than data:text/html,<input type="text"> on this theme/platform. This is true on my local platform, as well, actually (Ubuntu 19.04). This is weird & kinda broken.

I filed bug 1577256 on that underlying issue. For this bug here, I would strongly prefer a fails-if annotation over the patch's current fuzzy approach -- this is not really a fuzzy failure, but rather entirely different renderings -- so the ranges in the current patch (773 pixels differing by up to 202) amount to basically allowing the whole test to differ.

("fails-if" works particularly well if we can get a debian-specific condition here, as discussed in bug 1577053.)

I have been experimenting over the last few days, but haven't been able to zero in on anything that may differentiate the various Linux distributions.

The closest I believe I have gotten is to use the hh variable:

var hh = Cc[NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX + "http"].
                 getService(Ci.nsIHttpProtocolHandler);

which outputs:

[task 2019-09-06T02:23:02.129Z] 02:23:02     INFO - REFTEST INFO | ---------------------------------------
[task 2019-09-06T02:23:02.129Z] 02:23:02     INFO - REFTEST INFO | userAgent,appName,appVersion,vendor,vendorSub,product,productSub,platform,oscpu,language,misc
[task 2019-09-06T02:23:02.129Z] 02:23:02     INFO - REFTEST INFO | Mozilla/5.0 (X11; Linux x86_64; rv:71.0) Gecko/20100101 Firefox/71.0
[task 2019-09-06T02:23:02.129Z] 02:23:02     INFO - REFTEST INFO | Mozilla
[task 2019-09-06T02:23:02.129Z] 02:23:02     INFO - REFTEST INFO | 5.0
[task 2019-09-06T02:23:02.134Z] 02:23:02     INFO - REFTEST INFO | undefined
[task 2019-09-06T02:23:02.134Z] 02:23:02     INFO - REFTEST INFO | undefined
[task 2019-09-06T02:23:02.134Z] 02:23:02     INFO - REFTEST INFO | undefined
[task 2019-09-06T02:23:02.134Z] 02:23:02     INFO - REFTEST INFO | undefined
[task 2019-09-06T02:23:02.134Z] 02:23:02     INFO - REFTEST INFO | X11
[task 2019-09-06T02:23:02.136Z] 02:23:02     INFO - REFTEST INFO | Linux x86_64
[task 2019-09-06T02:23:02.136Z] 02:23:02     INFO - REFTEST INFO | undefined
[task 2019-09-06T02:23:02.136Z] 02:23:02     INFO - REFTEST INFO | rv:71.0
[task 2019-09-06T02:23:02.136Z] 02:23:02     INFO - REFTEST INFO | ---------------------------------------

I am not sure what other variables and/or values are present that could differentiate between ubuntu and debian.

Attachment #9088316 - Attachment is obsolete: true

Log of conversation in #developers on IRC:

egao> Anyone familiar with reftest conditions? I have tried to differentiate between distribution of Linux in manifest.jsm with no luck.
01:22 Ideas tried were to use userAgent string, http.oscpu and other variables but none of them are specific enough to Debian/Ubuntu/etc.
01:28 
<pulsebot> Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/3f4a21f5cb03 - Ed Lee - Bug 1557153 - Workaround UITour default permissions overridden by UNKNOWN_ACTION r=MattN a=lizzard
01:28 Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/34a6de6322f5 - Nika Layzell - Bug 1579437 - Check for discarded BrowsingContext in more places, r=farre a=lizzard
01:28 Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/18d2a997708a - Doug Thayer - Bug 1575610 - Ensure we only show spinner after 400ms r=mconley a=lizzard
01:28 Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/ffefc518e569 - Ryan Hunt - Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r=jrmuizel a=lizzard
01:29 Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/0cb1c338561d - Lina Cambridge - Bug 1577788 - Fix unique constraint errors when syncing bookmarks with tags. r=markh a=lizzard
01:29 Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/bc11f09acf75 - Boris Zbarsky - Bug 1568171.  Fix handling of load events if document.open() is called while async scripts are still loading for the original pageload.  r=smaug a=lizzard
01:29 
<emilio> egao: I'm somewhat familiar with the existing ones
01:29 
<pulsebot> Check-in: https://hg.mozilla.org/releases/mozilla-beta/rev/e5bdd7763d2d - Emilio Cobos Álvarez - Bug 1579788 - Downgrade a few assertions in beta / release as to avoid crashing there. r=masayuki a=lizzard
01:29 
<emilio> egao: but I don't think we differentiate distributions anywhere else... 
01:30 
<egao> emilio: I understand the current manifest conditions only determine broad categories i.e. windows, linux. But I don't know if something like distributions is even possible.
01:30 macOS seems to be an exception since the userAgent string reports the version number
01:31 
<Aryx> egao: is that our build or a different distribution? distribution.about would be interesting for the latter
01:31 
<egao> Aryx: not sure if I understand
01:32 
<Aryx> egao: is that about builds we compiled or from the distribution maintainer?
01:33 
<emilio> egao: It doesn't seem impossible... Not sure how reasonable, but we have runtime-switches for a few things. So if you can implement a C++ function that returns whether the distro is X you should be able to do it easily
01:33 
<egao> Aryx: this would be our own builds - I am looking to distinguish what distribution the instance of reftest is running against.
01:33 
<emilio> egao: you'd have to implement a function in nsDOMWindowUtils that returns the current distro and use it like https://searchfox.org/mozilla-central/rev/a777ff11b6d700a698c61e5bd17e73b044304494/layout/tools/reftest/manifest.jsm#447
01:34 egao: but another question is, why do reftests differ between distros? gtk themes?
01:34 ⇐ cbaica and vladimiroff quit  
01:34 
<emilio> egao: that seems like a much easier thing to test for if so
01:34 
<egao> emilio: root cause of this request is that I'm working to set up migration path from ubuntu 16.04 (unity) to debian 10 (GNOME)
01:35 tests like https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/HOu3aebxT1aETc83in3XNA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1 consistently fail because of the difference in padding of the fields
01:35 ⇐ jfkthame quit (jfkthame@moz-doeqqn.range86-179.btcentralplus.com) Quit: This computer has gone to sleep
01:35 
<emilio> egao: right, but the compositor is not important right? I guess the difference is the default ubuntu gtk theme vs. the debian one (Adwaita I guess) right?
01:36 
<egao> emilio: perhaps, I admit I am not as familiar as I should be regarding this.
01:36 
<emilio> egao: I'm pretty sure the field padding is due to gtk theme differences
01:37 
<pulsebot> Check-in: https://hg.mozilla.org/integration/autoland/rev/80a073621c53 - Hiroyuki Ikezoe - Bug 1541705 - Factor out functionalities for obsversing animation style counts into testcommon.js. r=boris
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/689df124ac2c - Hiroyuki Ikezoe - Bug 1541705 - Introduce remote document rect. r=botond
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/fb5bcfad4822 - Hiroyuki Ikezoe - Bug 1541705 - Notify areas of out-of-process subframes from the compositor to the corresponding process. r=botond
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/dd526a39c352 - Hiroyuki Ikezoe - Bug 1541705 - Introduce nsLayoutUtils::FrameIsScrolledOutOfViewInCrossProcess and use it for the check whether animating element is scrolled out of view or not. r=botond,boris
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/481dbebe905c - Hiroyuki Ikezoe - Bug 1541705 - Introduce nsLayoutUtils::FrameIsMostlyScrolledOutOfViewInCrossProcess. r=botond
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/10d08f48e097 - Hiroyuki Ikezoe - Bug 1541705 - Try to see whether the target frame is scrolled out of view in out-of-process iframe if we couldn't walk up the frame tree. r=surkov
01:37 Check-in: https://hg.mozilla.org/integration/autoland/rev/4880c3a309c9 - Hiroyuki Ikezoe - Bug 1541705 - Test for offscreen image in out-of-process iframe. r=tnikkel
01:37 
<emilio> egao: so rather than detecting distros (you could do it with an environment variable when run on automation at least, I guess)
01:38 egao: it may be easier to just get the gtk theme
01:38 egao: and use it in the reftest conditions
01:39 egao: so adding something like `nsIDOMWindowUtils::GetCurrentGtkTheme` or such, and do something like https://searchfox.org/mozilla-central/rev/a777ff11b6d700a698c61e5bd17e73b044304494/toolkit/xre/nsAppRunner.cpp#2709
01:39 egao: I'm ~sure that'd return different values in ubuntu vs. debian
01:40 egao: though there are other people more familiar than me with this stuff, like stransky 
01:40 
<egao> emilio: thanks, good ideas. I do not know C++ so this will be an interesting exercise, but hopefully I can make something work out
01:41 
<emilio> egao: np, let me know if you need help, I'm happy to :)
01:41 → vladimiroff joined (vladimiroff@moz-5iid5r.pon.spectrumnet.bg)
01:42 
<heycam> egao: I wonder if you could somehow get the result of running `lsb_release -is` into there
01:42 which might be more reliable than checking the theme
01:43 
<stransky> egao, yes, padding/borders depend on actual gtk theme. better don't test that or check is under some sane values. Also I'd always use a default Gtk theme (Adwaita).
01:44 
<egao> I'm feeling that this is quickly getting over my head
01:44 → ErikRose joined  ⇐ florian quit  
01:47 
<pulsebot> Check-in: https://hg.mozilla.org/integration/autoland/rev/11b6d1bbafe7 - Masayuki Nakano - Bug 1574852 - part 98: Move `HTMLEditRules::WillAbsolutePosition()` and `HTMLEditRules::WillRemoveAbsolutePosition()` to `HTMLEditor` r=m_kato
01:47 Check-in: https://hg.mozilla.org/integration/autoland/rev/dc0f654c9ccd - Masayuki Nakano - Bug 1574852 - part 99: Move `HTMLEditRules::WillRelativeChangeZIndex()` to `HTMLEditor` r=m_kato
01:47 Check-in: https://hg.mozilla.org/integration/autoland/rev/886ccc65f1c9 - Masayuki Nakano - Bug 1574852 - part 100: Get rid of `TextEditRules::DidDeleteSelection()` r=m_kato
01:47 
<emilio> egao: Is there a bug tracking this btw?
01:48 egao: I'm happy to help out with this, though probably would have to be after tpac
01:48 
<pulsebot> Check-in: https://hg.mozilla.org/integration/autoland/rev/667132500392 - Emilio Cobos Álvarez - Bug 1579788 - Downgrade a few assertions in beta / release as to avoid crashing there. r=masayuki
01:48 
<egao> emilio: yes, it is in bug 1575969
01:48 
<heycam> masayuki: 💯
01:48 ⇐ ErikRose quit (erikrose@moz-s9e.nt0.69.71.IP) Ping timeout: 121 seconds
01:48 
<emilio> lol
01:50 egao: if you don't get it to work feel free to ni? me in that bug and I'll be happy to make it work, or give it a try at least ;)
01:51 
<egao> emilio: thanks, I will try again

Instead of trying to figure out a reftest manifest condition specific to Debian, I tried another approach that stransky suggested, which is to install the same GTK theme as Ubuntu 16.04 on the Debian 10 image, then activate the theme prior to test execution.

Ambiance can be obtained from wget https://launchpad.net/~ravefinity-project/+archive/ubuntu/ppa/+files/ambiance-flat-colors_16.04.1~xenial~NoobsLab.com_all.deb.
Installation to Debian 10 docker image is successful:

[task 2019-09-12T11:11:09.754Z] 2019-09-12 11:11:09 (1.80 MB/s) - 'ambiance-flat-colors_16.04.1~xenial~NoobsLab.com_all.deb' saved [1117378/1117378]
[task 2019-09-12T11:11:09.754Z] 
[task 2019-09-12T11:11:09.755Z] dpkg -i ambiance-flat-colors_16.04.1~xenial~NoobsLab.com_all.deb
[task 2019-09-12T11:11:09.789Z] Selecting previously unselected package ambiance-flat-colors.
[task 2019-09-12T11:11:09.875Z] (Reading database ... 165878 files and directories currently installed.)
[task 2019-09-12T11:11:09.881Z] Preparing to unpack ambiance-flat-colors_16.04.1~xenial~NoobsLab.com_all.deb ...
[task 2019-09-12T11:11:09.882Z] Unpacking ambiance-flat-colors (16.04.1~xenial~NoobsLab.com) ...
[task 2019-09-12T11:11:11.551Z] Setting up ambiance-flat-colors (16.04.1~xenial~NoobsLab.com) ...

Activation of the theme is also successful:

[task 2019-09-12T11:33:05.352Z] + gsettings set org.gnome.desktop.interface gtk-theme Ambiance

However, the reftest analyzer shows that input field height still varies between the TEST and REFERENCE images:
https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/fSsRn4ZWTQySUpFo8fVBsg/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

So it would seem that this change does not address the discrepancy in the images for Reftest, at least.

Hmm, I'm skeptical that the theme actually applied effectively.

The reftest screenshot in comment 9 shows a blue coloring on the widgets (outline & highlight color), and that's not how the Ubuntu 16.04 theme looks.

Ubuntu 16.04's Ambiance theme primarily uses orange as a highlight/focus color. Here's a screenshot showing some dialogs from Ubuntu 16.04 as supporting evidence for that. If we get the theme correctly applied, I'd expect we'd see widgets with an outline/highlight-color that looks like the orange in this screenshot.

Right, I see what you're saying and it makes sense, but at this point I'm not sure what else I could do. For the GTK theme attempt, I followed the instructions in https://www.ubuntupit.com/install-ambiance-radiance-flat-colors-gtk-themes-debian-ubuntu-linux-mint/.

Could it be possible that the step of redefining the test parameter for the newer GTK theme in Debian 10 has to be taken, given that Ubuntu 16.04 and its Ambiance theme won't be around forever?

(In reply to Edwin Takahashi (:egao, :etakahashi [leave 9/16-10/15]) from comment #11)

Could it be possible that the step of redefining the test parameter for the newer GTK theme in Debian 10 has to be taken, given that Ubuntu 16.04 and its Ambiance theme won't be around forever?

Yeah -- if we can't get the Ubuntu 16.04 theme reliably working on Debian, then our options are:
(1) fix bug 1577256 (which we should do but probably isn't super high-priority on its own)
(2) add a test parameter for these debian testers, and annotate the test as fails-if(debian)
(3) Just consider the test broken on Linux, and annotate it as random-if(gtkWidget) so that it can continue to pass on our Ubuntu 16.04 testruns and continue to fail on our debian testruns.

Option #1 is ideal, though I don't know if we'll get to it in time to be useful.
Option #2 is strongly preferable over option #3, because it lets us continue to encode strict expectations about the test rendering (and it will let us actually detect if the situation changes).

(In reply to Daniel Holbert [:dholbert] from comment #12)

(In reply to Edwin Takahashi (:egao, :etakahashi [leave 9/16-10/15]) from comment #11)

Could it be possible that the step of redefining the test parameter for the newer GTK theme in Debian 10 has to be taken, given that Ubuntu 16.04 and its Ambiance theme won't be around forever?

Yeah -- if we can't get the Ubuntu 16.04 theme reliably working on Debian, then our options are:
(1) fix bug 1577256 (which we should do but probably isn't super high-priority on its own)
(2) add a test parameter for these debian testers, and annotate the test as fails-if(debian)
(3) Just consider the test broken on Linux, and annotate it as random-if(gtkWidget) so that it can continue to pass on our Ubuntu 16.04 testruns and continue to fail on our debian testruns.

Option #1 is ideal, though I don't know if we'll get to it in time to be useful.
Option #2 is strongly preferable over option #3, because it lets us continue to encode strict expectations about the test rendering (and it will let us actually detect if the situation changes).

I had a chance to revisit this issue after returning from my parental leave.

I did spend the two weeks prior to going on leave asking about this, and while there appears to be technical avenues to add a debian-specific criteria into reftest manifests, it is way over my abilities to understand.

I was able to reduce the number of failures in the reftest suites with changes to the docker container, but we do still see slight differences in the way HTML elements are rendered between debian10 and ubuntu1604.

Closing this bug due to redirection of efforts from debian10 to ubunu1804 image, and this bug is not reproducible on the ubuntu1804 image.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: