Closed Bug 818128 Opened 9 years ago Closed 7 years ago

Failure in testAwesomeBar/testVisibleItemsMax.js | Number of visible rows should equal 6


(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)



(firefox23 wontfix, firefox24 wontfix, firefox25 wontfix, firefox26 wontfix, firefox30 wontfix, firefox31 wontfix, firefox32 fixed, firefox33 fixed, firefox34 fixed, firefox-esr17 unaffected, firefox-esr24 wontfix, firefox-esr31 fixed)

Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- wontfix
firefox30 --- wontfix
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
firefox34 --- fixed
firefox-esr17 --- unaffected
firefox-esr24 --- wontfix
firefox-esr31 --- fixed


(Reporter: AlexLakatos, Assigned: andrei)




(Whiteboard: [mozmill-test-failure])


(5 files, 1 obsolete file)

TEST: /testAwesomeBar/testVisibleItemsMax.js::testVisibleItemsMax
ERROR: Number of visible rows should equal 6
WHEN: 12-04-2012
FIRST: 12-02-2012
Priority: P2 → P3
Did not happen since 12/04
Closed: 9 years ago
Resolution: --- → WORKSFORME
Firefox 23.0 (23.0, zh-TW, 20130718163513) on Linux Ubuntu 12.10 (x86):
Resolution: WORKSFORME → ---
(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):
> 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.
Summary: Failure in testAwesomeBar/testVisibleItemsMax.js | Number of visible rows should equal 6 → [zh-TW] Failure in testAwesomeBar/testVisibleItemsMax.js | Number of visible rows should equal 6
Firefox 23.0 (23.0, zh-TW, 20130722172257) - Linux Ubuntu 12.10 (x86):
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.
Flags: needinfo?(andreea.matei)
Looking into it.
Assignee: nobody → andrei.eftimie
Flags: needinfo?(andreea.matei)
Hight failure rate on Linux. (9 out of 10)
Fails across all branches except ESR17.
OS: Windows XP → Linux
Hardware: x86 → All
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.
No longer depends on: 583683
Attached file testcase.js
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.
Depends on: 901946
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:

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):
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: 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)
Well scratch the mozilla-release pushlog link, I don't think we can find any useful information there :(
Attached patch skip.patchSplinter Review
Skip patch for Beta and Release
Attachment #801646 - Flags: review?(andreea.matei)
Comment on attachment 801646 [details] [diff] [review]

Review of attachment 801646 [details] [diff] [review]:

Disabled: (beta) (release)

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:  

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:

The assert for 5 or 6 rows doesn't seem that reliable to me as we could miss a regression this way.
Attached patch fix.patch (obsolete) — Splinter Review
Let's test for the actual algorithm that sets the height for the AutoComplete toolbar, which is first row height * maxrows [1] 

This patch does just that. Checking this still leaves us asserting that the number of visible rows is maxrows.

Attachment #8476589 - Flags: review?(andreea.matei)
Comment on attachment 8476589 [details] [diff] [review]

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-
Attached patch fix2.patchSplinter Review
Actually had another typo to fix.
Attachment #8476589 - Attachment is obsolete: true
Attachment #8477391 - Flags: review?(andreea.matei)
Comment on attachment 8477391 [details] [diff] [review]

Review of attachment 8477391 [details] [diff] [review]:
----------------------------------------------------------------- (default)

For backporting you'll need to also unskip the test.
Attachment #8477391 - Flags: review?(andreea.matei) → review+
Attached patch backport.patchSplinter Review
Backported to aurora, there the test was enabled. (aurora)

From beta down, I'm adding the patch.
Attachment #8481254 - Flags: review?(andrei.eftimie)
Comment on attachment 8481254 [details] [diff] [review]

Review of attachment 8481254 [details] [diff] [review]:

Works great. (mozilla-beta)
Attachment #8481254 - Flags: review?(andrei.eftimie)
Attachment #8481254 - Flags: review+
Attachment #8481254 - Flags: checkin+
Transplanted: (mozilla-release) (mozilla-esr31)

We should be good here.
Closed: 9 years ago7 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped][Blocked by bug 901946] → [mozmill-test-failure]
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.