Closed Bug 818128 Opened 7 years ago Closed 6 years ago
Failure in test
Awesome Bar/test Visible Items Max .js | Number of visible rows should equal 6
135.78 KB, image/jpeg
1.67 KB, patch
|Details | Diff | Splinter Review|
1.85 KB, patch
|Details | Diff | Splinter Review|
2.59 KB, patch
|Details | Diff | Splinter Review|
TEST: /testAwesomeBar/testVisibleItemsMax.js::testVisibleItemsMax ERROR: Number of visible rows should equal 6 WHEN: 12-04-2012 FIRST: 12-02-2012 FREQ: 2
Did not happen since 12/04
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Firefox 23.0 (23.0, zh-TW, 20130718163513) on Linux Ubuntu 12.10 (x86): http://mozmill-release.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d611e12a2
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #2) > Firefox 23.0 (23.0, zh-TW, 20130718163513) on Linux Ubuntu 12.10 (x86): > http://mozmill-release.blargon7.com/#/functional/report/ > 180cf2548ef2865af3ae441d611e12a2 Hm, most likely we should file separate bugs in the future if the problem occurs only for localized builds of firefox or a special locale. This failure we haven't seen for a really long time so I think there might be different causes. Given that the en-US failure never re-occurred lets more this bug to cover the zh-TW locale.
Firefox 23.0 (23.0, zh-TW, 20130722172257) - Linux Ubuntu 12.10 (x86): > http://mozmill-release.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61a1babf
Andreea, it would be great if someone from the SV team could have a quick check what's going on here. We face this issue each time we run tests with the zh-TW locale.
Looking into it.
Assignee: nobody → andrei.eftimie
Status: REOPENED → ASSIGNED
Hight failure rate on Linux. (9 out of 10) Fails across all branches except ESR17.
zh-TW locale returns 7 visible rows in > autoCompleteResultsList.getNode().getNumberOfVisibleRows() I can see that the 7th item has about 10px more of visible area. You can clearly see that in the attached picture wich contains en-US and zh-TW overlaid at 50% opacity. This might be a genuine regression in Firefox if the intention is to only see 6 complete rows, not more.
Looks like we actually depend on bug 583683. So the test might have to be disabled or only run parts of the checks.
Depends on: 583683
I'm not sure 583683 is to blame. The setting is still set at 6 items. en-US the parent element has a height of 246px (41 * 6) zh-TW (on OSX is still 246) the parent element reports has a total height of 282px (47 * 6) Each item in the list is 5px taller on Linux zh-TW (might be glyph rendering takes up more vertical space or something similar). Manually I can't make the autocomplete popup show me the extra item. It is consistent at 282px (47 * 6) without space underneath. I'll try to reproduce manually (make sure I have the same pages visited in the same order) or have a small testcase, this looks like some rendering issue in FF to me.
I've attached a simplified testcase. Run it with mozmill as usual: > mozmill -t testcase.js -b %path/to/firefox/binary% I've left some dump statements for the following: 1) the height of all Autocomplete items 2) cumulative height of the first 6 items 3) total height of the Autocomplete window 2) and 3) should be equal They are with a en-US build They differ with a zh-TW build This looks like a regression in Firefox.
Happened today on Ubuntu 12.10 (x86) with Beta zh-TW http://mozmill-release.blargon7.com/#/functional/report/b7ef1fb3d9703aeaf2c46e07d28385e5
Thanks for the link Anthony. I would have expected to see this fail on languages with a non-latin script. I have however not been able to reproduce this on OSX. Will check these out. This looks to have regressed somewhere between Firefox 18 and Firefox 19, I'll have some compiling to do.
defintiely not Zh-TW only. This failure appears on many of the locales now that we're running all locales at least once per functional pass against betas.
Summary: [zh-TW] Failure in testAwesomeBar/testVisibleItemsMax.js | Number of visible rows should equal 6 → Failure in testAwesomeBar/testVisibleItemsMax.js | Number of visible rows should equal 6
I have been able to reproduce the failure on OSX. While zh-TW works good on my machine, or fails in exactly the same way. Windows does not seem affected, only *nix systems.
Affected languages that we are aware ATM: ak ar as ja mai ml mr or ta ta-LK te zh-TW This is clearly not a localisation issue per-se. Possibly a glyph rendering, line-height calculation, flex-box height issue.
Happened again today on Linux Ubuntu 13.04 (x86) with Firefox 24.0 (24.0, ja, 20130902131354): http://mozmill-release.blargon7.com/#/functional/report/3c3cd991de01b704f3f65783a1c3b9b1
Happened again on Ubuntu 12.04 and 13.04 with Beta 'ja' locale. http://mozmill-release.blargon7.com/#/functional/report/fb97b6210ae70da1b9ace6744558a450 http://mozmill-release.blargon7.com/#/functional/report/fb97b6210ae70da1b9ace67445589a04
approximately 50% of all failures in the runs for beta have been this bug.
P1! Lets get this fixed to make QA happy!
Priority: P3 → P1
We have filed bug 901946 as this is most probably a Firefox Regression. THe only solution to manage these failures right now would be to skip the test. We only fail on certain 'exotic' locales, we could implement a blacklist of locales we want the test skipped... or skip it across all locales (this may be to invasive). On fixing the real issue: - the regression was introduced between Firefox 18 and Firefox 19 - I have gotten closer to building a localised Firefox (using compare-locale, thanks :pike, I have a list of changes I need to do for the l10n repo, but it is time-consuming, and there is lots of ground to be covered) - in theory we have a pushlog for mozilla-release: http://hg.mozilla.org/releases/mozilla-release/pushloghtml?fromchange=8efe34fa2289&tochange=20238b786063 but the webserver fails to deliver anything (probably to big) To summmarize: 1) I'll provide a blacklist based skip-patch 2) I'll continue trying to narrow down the origin of the regression (it would be great to have older localised builds of FF)
Happened again on Mac with beta 'ko' locale. http://mozmill-release.blargon7.com/#/functional/report/fb97b6210ae70da1b9ace67445776386
OS: Linux → All
Well scratch the mozilla-release pushlog link, I don't think we can find any useful information there :(
Skip patch for Beta and Release
Attachment #801646 - Flags: review?(andreea.matei)
Comment on attachment 801646 [details] [diff] [review] skip.patch Review of attachment 801646 [details] [diff] [review]: ----------------------------------------------------------------- Disabled: http://hg.mozilla.org/qa/mozmill-tests/rev/92af909ac97f (beta) http://hg.mozilla.org/qa/mozmill-tests/rev/f9463c6441de (release) Thanks.
Attachment #801646 - Flags: review?(andreea.matei) → review+
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mozmill-test-skipped]
Why has this been skipped across all locales and only on Beta and Release? This fails across all branches on specific locales. I've explained in comment 22 that we would need a blacklist of locales for which the test is skipped, I've worked today on compiling that list. We haven't seen failures on Nightly and Aurora because we didn't run this test against them. We will soonish, and this will fail on the same locales. I also don't understand why this had to be rushed before the merge (since we have prior established the branches are closed until the actual merge). Couldn't it have waited until tomorrow?
No, this was the most concerning point of QA which we discussed during the meeting today. So please read the meeting notes about it. We only skipped the test on those branches to stop failures for ondemand tests. We still have to work our a solution for all brnaches, but only keep beta and release skipped.
Actually ESR17 is not affected as the regression was introduced in Firefox 19
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure][mozmill-test-skipped][Blocked by bug 901946]
This test showed up as a failure email to mozmill-ci but with 0 failures. The report shows 3 tests were skipped including this one: http://mozmill-release.blargon7.com/#/functional/report/a3b8d1e9bea5a0fc05cd41dbf639a2a4 I'm not sure it's necessary to comment here on the skipped test but here goes.
That is, in a failure email from functional tests for Firefox 31.0 RC.
That means the testrun did not end properly, most likely a known issue we're facing with WSAEINVAL. Daniel is the duty this week and will retrigger such testruns.
Here we're also blocked by bug 901946 where I don't see a fix in the near future. Can we find a fix, like instead of checking 6 rows to check the line height x 6? Since that differs for some locales. For a fact we know we should have 6 rows and some locales have different heights for those rows. Something like it was done in bug 804968: https://hg.mozilla.org/mozilla-central/rev/0fc382c36b4c#l1.39 The assert for 5 or 6 rows doesn't seem that reliable to me as we could miss a regression this way.
Let's test for the actual algorithm that sets the height for the AutoComplete toolbar, which is first row height * maxrows  This patch does just that. Checking this still leaves us asserting that the number of visible rows is maxrows.  http://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/autocomplete.xml#1065-1073
Attachment #8476589 - Flags: review?(andreea.matei)
Comment on attachment 8476589 [details] [diff] [review] fix.patch Review of attachment 8476589 [details] [diff] [review]: ----------------------------------------------------------------- Just a small nit. ::: firefox/tests/functional/testAwesomeBar/testVisibleItemsMax.js @@ +69,5 @@ > var maxRows = locationBar.urlbar.getNode().getAttribute("maxrows"); > + > + // Bug 901946 > + // Because the height is not compted dynamically but is row1 height * maxrows > + // getNumberOfVisibleRows() returns a different number then max for certain nit: than
Attachment #8476589 - Flags: review?(andreea.matei) → review-
Actually had another typo to fix.
Comment on attachment 8477391 [details] [diff] [review] fix2.patch Review of attachment 8477391 [details] [diff] [review]: ----------------------------------------------------------------- http://hg.mozilla.org/qa/mozmill-tests/rev/e05203fa117d (default) For backporting you'll need to also unskip the test.
Attachment #8477391 - Flags: review?(andreea.matei) → review+
Backported to aurora, there the test was enabled. http://hg.mozilla.org/qa/mozmill-tests/rev/6b20de7dabbe (aurora) From beta down, I'm adding the patch.
Comment on attachment 8481254 [details] [diff] [review] backport.patch Review of attachment 8481254 [details] [diff] [review]: ----------------------------------------------------------------- Works great. https://hg.mozilla.org/qa/mozmill-tests/rev/af566e58a984 (mozilla-beta)
Transplanted: https://hg.mozilla.org/qa/mozmill-tests/rev/4bcdf37d9466 (mozilla-release) https://hg.mozilla.org/qa/mozmill-tests/rev/f885cdd997bf (mozilla-esr31) We should be good here.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 6 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped][Blocked by bug 901946] → [mozmill-test-failure]
You need to log in before you can comment on or make changes to this bug.