Closed Bug 834724 Opened 12 years ago Closed 12 years ago

2 reftest failures on ubuntu 12.04 ec2 vm machines in forms

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla22

People

(Reporter: jmaher, Assigned: jmaher)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

make reference resizer scale to scroll bar width
1.68 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
random-if textarea-rtl as it fails on ubuntu right now (1.2)
996 bytes, patch
jmaher
: review+
Details | Diff | Splinter Review
mark textarea-rtl.html as fails-if instead of random-if (1.0)
1007 bytes, patch
karlt
: review+
Details | Diff | Splinter Review
from this log file (https://tbpl.mozilla.org/php/getParsedLog.php?id=19103276&tree=Cedar), we have 2 reftest form failures: 13:43:07 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/forms/textarea-resize-background.html | image comparison (==), max difference: 122, number of differing pixels: 65 13:43:08 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/forms/textarea-rtl.html | image comparison (!=) it might help to see the tbpl toolchain for reftest analyzer: https://tbpl.mozilla.org/?tree=Cedar&rev=706f8a9ee780 these fail consistently, we need to fix or disable these somehow as we are looking to run the majority of our unittests on ec2 VMs.
Blocks: 834725
This test was added in bug 554810, and is using a reference image. Do we get the resizer from the OS widget layer these days? If yes, no wonder it fails.
Blocks: 554810
The resizer looks a little different with different GTK themes, so themes are involved somehow, but I'm not seeing moz_gtk_resizer_paint being called. I'm wondering whether it might be the same resizer being scaled differently in the different themes. Anyone know why the second reftest run gave different results?
The (un-themed) resizer icon is 15x15 but is resized to the scrollbar thickness, which is themed. http://hg.mozilla.org/mozilla-central/annotate/20bbf73921f4/layout/generic/nsGfxScrollFrame.cpp#l3546 Perhaps the test could be made to pass if the reference/test can be made scrollable/non-scrollable consistently, but perhaps we should reconsider whether the resizer should not be scaled.
Blocks: 837021
if you guys have a tip on how I can determine what theme is being used or how I could [un]enforce scrollable/non-scrollable I would be happy to test on one of these new VM machines.
"overflow: hidden;" in the textarea style should make textarea-resize-background.html pass. When the scrollbars are styled (always perhaps) and the resizer is not, then there is an argument for scrolling the non-native resizer to match the scrollbars, so the current behaviour may be fine in this test. Just get Enn's feedback that he's happy that the test is still testing what he wanted to test.
I mean "When the scrollbars are native (always perhaps) and the resizer is not ..."
... there is an argument for scaling the non-native resizer ...
adding in the "overflow: hidden;" fixes the textarea-resize-background.html test! This is great stuff. we still have a failure with textarea-rtl.html.
I can reproduce the lack of difference between textarea-rtl.html and textarea-no-resize.html with the oxygen-gtk theme (but not the default theme, nor Clearlooks). textarea-ltr.html also has no resizer visible. I'll investigate further later in the week.
gtk_preview (from kde-gtk-config) doesn't draw a resizer with oxygen-gtk either. I haven't found any other GTK apps using resizers so I don't know what else you could use to test. If the theme chooses not to draw a resizer, then the user needs to mouse over to look for a cursor change, but that may not be a bug. However, != textarea-ltr.html textarea-no-resize.html seems to be passing on Ubuntu. Can you attach a screenshot of textarea-ltr.html, please?
sorry, I meant textarea-rtl.html is failing. This fails for me on my local Ubuntu 12.04 box. I can fails-if(gtk2Widget) and call it good. I am happy to try other experiments though.
Yes, it is odd that textarea-rtl.html matches textarea-no-resize.html (i.e. each has no resizer) but textarea-ltr.html doesn't match, implying that it is showing a resizer. I'm curious about what the resizer looks like.
I have the test/reference images for textarea-ltr and textarea-rtl in the form of a log file: http://people.mozilla.org/~jmaher/forms.log you can see the png files embedded in there for what we are comparing against.
Thanks, textarea-ltr.html looks as expected. I can reproduce with the Ambience theme, so will look into it.
awesome, thanks!
The murrine theme engine (used for Ambiance) doesn't support the resizers used http://git.gnome.org/browse/murrine/tree/src/murrine_style.c?id=64a52395a6d5e7500205c8fe97589314b9dc432e#n1880 Perhaps Ubuntu uses a different theme for LTR locales, or perhaps not many apps use GtkStatusBar. This is an issue with the theme engine, so we don't need to fix our code, so just mark the test random-if(gtk2Widget). If the test can be marked fails-if(gtk2Widget) when all tests are run on Ubuntu, please, then we'll notice when we can expect the test to pass again.
patch as requested. We will [leave open] this bug and when we are not running tests on fedora anymore, we will adjust to a fails-if.
Assignee: nobody → jmaher
Attachment #711238 - Flags: review?(karlt)
Whiteboard: [leave open]
Comment on attachment 711238 [details] [diff] [review] fix 2 remaining forms tests on ubuntu (1.0) Neil, are you happy with the overflow: hidden in textarea-resize-background? See comment 5.
Attachment #711238 - Flags: review?(karlt)
Attachment #711238 - Flags: review+
Attachment #711238 - Flags: feedback?(enndeakin)
Comment on attachment 711238 [details] [diff] [review] fix 2 remaining forms tests on ubuntu (1.0) >-skip-if(B2G) fails-if(Android) != textarea-rtl.html textarea-no-resize.html >+skip-if(B2G) fails-if(Android) random-if(gtk2Widget) != textarea-rtl.html textarea-no-resize.html Can you add "# bug 834724" to the end of this line, please?
updated with the comment to reference this bug.
Attachment #711238 - Attachment is obsolete: true
Attachment #711238 - Flags: feedback?(enndeakin)
Attachment #711546 - Flags: review+
Attachment #711546 - Flags: feedback?(enndeakin)
Comment on attachment 711546 [details] [diff] [review] fix 2 remaining forms tests on ubuntu (1.1) I'm pretty sure that this test is intending to test nsNativeTheme::IsWidgetStyled which tests that the resizer is displayed properly when the textarea is scrollable.
Attachment #711546 - Flags: feedback?(enndeakin) → feedback-
Thanks Neil. Would this be a bug we found on the Ubuntu theme being used? Any tips on how to proceed so we can move reftest over to ubuntu vm?
The test shouldn't be using the native resizer image. This is because a scrollable container that has an overridden border or background should cause nsNativeTheme::IsWidgetStyled to return true and prevent the native theme from being used. Is the difference only that the scrollbar has a different size on the native themes that are failing? Does the reference for the test need to adapt to this?
you can see the difference via reftest analyzer in this push: https://tbpl.mozilla.org/?tree=Cedar&rev=2d816d69761b from looking at the different resizers, it appears the reference one is a bit more scaled up. I get the same error when I run it on my local on the metal ubunut 12.04 desktop, so this isn't an issue with a VM. Signs are pointing at a need to update the resizer.png image. Can we confirm that, and what needs to be done to do that?
(In reply to Neil Deakin from comment #23) > Is the difference only that the scrollbar has a different size on the native > themes that are failing? Yes, the resizer is scaled to the same size as the scrollbar thickness. > Does the reference for the test need to adapt to this? I don't know whether or not the available area might be available to JS to determine the thickness of scrollbars to resize to the resizer.png. (In reply to Neil Deakin from comment #21) > I'm pretty sure that this test is intending to test > nsNativeTheme::IsWidgetStyled which tests that the resizer is displayed > properly when the textarea is scrollable. I thought the test was just testing that when there was a background color specified on the textarea, the resizer was not the native resizer. But if you intended to test more than that, and the size of the scrollbar is not available to JS, then perhaps we should disable on Linux/GTK.
next steps are either: 1) accept patch as is 2) fix the resizer.png in the theme we care about 3) disable the test for linux/GTK via Makefile Can we make a decision on one of these steps so we can move forward sometime this week and get the reftests ready to run on Ubuntu.
The non-native resizer does not look as nice when scaled, and may be hard to see on grey backgrounds, but this keeps the test testing what it is currently testing.
Attachment #713280 - Flags: review?(enndeakin)
Attachment #713280 - Flags: review?(enndeakin) → review+
I will upload a new patch without the changes to textarea-resize-background.html, only the random-if for the textarea-rtl test case.
two patches which should get us to green. I will work on landing these tonight if somebody else doesn't beat me to it.
Attachment #711546 - Attachment is obsolete: true
Attachment #713582 - Flags: review+
I am seeing this when we are running in automation: REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/reftest/tests/layout/reftests/forms/textarea-resize-background.html | image comparison (==), max difference: 79, number of differing pixels: 90 It is random (about 25%) so we might want to adjust the manifest. Here is the log: https://tbpl.mozilla.org/php/getParsedLog.php?id=19740422&tree=Cedar here is a link to the tbpl instance for reftest-analyzer: https://tbpl.mozilla.org/?tree=Cedar&rev=ca5146f70fb1
(In reply to Joel Maher (:jmaher) from comment #0) > https://tbpl.mozilla.org/?tree=Cedar&rev=706f8a9ee780 Here, before changes to the reftest, there are (currently) 3 reftest runs for Ubuntu64 opt. The second run gave different results for textarea-resize-background.html, and both textarea-rtl.html and underline-button-2.html passed (despite failing in the first and third runs). The textarea-resize-background.html images in the first and third runs there are consistent with what I see with the Ambience theme. I can't explain how textarea-rtl.html could pass with the Ambience theme. (In reply to Joel Maher (:jmaher) from comment #31) > I am seeing this when we are running in automation: > REFTEST TEST-UNEXPECTED-FAIL | > file:///builds/slave/test/build/reftest/tests/layout/reftests/forms/textarea- > resize-background.html | image comparison (==), max difference: 79, number > of differing pixels: 90 > > It is random (about 25%) [...] > here is a link to the tbpl instance for reftest-analyzer: > https://tbpl.mozilla.org/?tree=Cedar&rev=ca5146f70fb1 The runs that are failing there show a resizer the same as the second run on 706f8a9ee780. What is different about the machines that ran the second run on 706f8a9ee780 and the failing runs on ca5146f70fb1? textarea-rtl.html suggests a different theme. underline-button-2.html suggests a different font or font options. Is there a way to request that the same machine runs the failing tests again, so we can determine whether this is truly intermittent or a variation across machines?
textarea-rtl should be random-if right now, when we move over to ubuntu only it will be fails-if. If there is a different theme that is concerning. As far as I know the machines are identical, with the exception that 32 bit and 64 bit will be different in a variety of ways. I saw via vncviewer the theme looks a bit different, but in general it was the same. The font options will be different between 32 and 64 as well as a local desktop. Rail, is there anything you could add to this? On build-system, we have a run which has similar results: https://tbpl.mozilla.org/?tree=Build-System&rev=aceeea086ccb
Does "tst-linux64-ec2-023" identify a particular VM that should be exactly the same each time "tst-linux64-ec2-023" is reported? If so, we can schedule many runs and wait until we get multiple from the same VM. (In reply to Joel Maher (:jmaher) from comment #33) > textarea-rtl should be random-if right now, when we move over to ubuntu only > it will be fails-if. It is random-if() right now. That is why the textarea-rtl passes, on the same runs as textarea-resize-background.html failures, are not showing up in the summaries.
(In reply to Joel Maher (:jmaher) from comment #33) > If there is a different theme that is concerning. As far as I know the > machines are identical, with the exception that 32 bit and 64 bit will be > different in a variety of ways. I saw via vncviewer the theme looks a bit > different, but in general it was the same. > > The font options will be different between 32 and 64 as well as a local > desktop. Rail, is there anything you could add to this? Ubuntu 32-bit and 64-bit machines are identical (package base wise). The use the same procedure for initial installation and puppetization. (In reply to Karl Tomlinson (:karlt) from comment #34) > Does "tst-linux64-ec2-023" identify a particular VM that should be exactly > the same each time "tst-linux64-ec2-023" is reported? Yes, names are consistent per VMs.
tst-linux64-ec2-050 and tst-linux64-ec2-097 have both given each of the different sets of results, so the difference is not between the VMs. tst-linux64-ec2-050 19607307:a 19682197:b tst-linux64-ec2-097 19606546:a 19635089:b where a is the expected result with Ambience and b is the different result with unknown cause.
could it be a random failure? Maybe a timing issue? We are really close here, I would rather fix it right if we can.
What seems to preclude a timing issue (or at least a timing issue directly related to these tests) is that there are a number things that all happen together, if they happen. Another clue is that, when the tests fail, dir_auto-input-EN-L.html does not produce these warnings, which it usually does produce: 09:48:18 INFO - WARNING: NS_ENSURE_SUCCESS(result, result) failed with result 0x80004005: file ../../../../editor/libeditor/base/nsEditor.cpp, line 3904 09:48:18 INFO - WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file ../../../../editor/libeditor/text/nsTextEditRules.cpp, line 410 I don't know what that tells us. (In reply to Joel Maher (:jmaher) from comment #33) > As far as I know the > machines are identical, with the exception that 32 bit and 64 bit will be > different in a variety of ways. I saw via vncviewer the theme looks a bit > different, but in general it was the same. Might be worth investigating further there, as things should be exactly the same.
I'd like to see the rendering that is making textarea-rtl.html pass unexpectedly. Can this be marked fails-if on cedar, or is there a way to run these tests on try?
textarea-rtl fails on all the ubuntu vms, but right now it is run on fedora as well which passes, so for the short term it is random-if.
(In reply to Joel Maher (:jmaher) from comment #40) > textarea-rtl fails on all the ubuntu vms, Not always. When textarea-resize-background.html fails on Ubuntu VMs, textarea-rtl passes. I want to see how textarea-rtl and its reference are rendered when it passes. Is there try chooser syntax to request the Ubuntu reftests to be run on try? If so, I could push a patch with fails-if for that test, and I expect I'd get images in the log. Or is there another way to get images for the unexpected textarea-rtl results?
Depends on: 841970
The discovery of bug 841970 means that I can now reproduce the textarea-resize-background.html failure by using the default GTK theme instead of Ambience. The resizer is a different size when scrollbars are visible. I can look into this.
Depends on: 842468
textarea-resize-background.html should be fixed, even with the default GTK theme with changes in bug 842468. textarea-rtl will continue to pass sometimes until bug 841970 is resolved.
fix this to be a permanent solution. I understand there is a chance this will pass with the incorrect theme, but that is well under way to being resolved.
Attachment #720003 - Flags: review?(karlt)
Whiteboard: [leave open]
Comment on attachment 720003 [details] [diff] [review] mark textarea-rtl.html as fails-if instead of random-if (1.0) Assuming this will be landed after reftests move from Fedora to Ubuntu.
Attachment #720003 - Flags: review?(karlt) → review+
https://hg.mozilla.org/mozilla-central/rev/491d60eb44bf
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
textarea-rtl.html in #74 there is the result we get when GTK's default theme gets used instead of Ambience.
I realized after I made the comment that there is no theme applied. This should be fixed in the next build. Thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: