bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

Mozmill tests for cut-off elements should not report failure for scrollable direction



Mozilla QA
Mozmill Tests
6 years ago
5 years ago


(Reporter: whimboo, Assigned: Andrei Eftimie)


Dependency tree / graph

Firefox Tracking Flags

(firefox22 fixed, firefox23 fixed, firefox24 fixed, firefox25 fixed, firefox-esr17 fixed)


(Whiteboard: [mozmill-l10n][mentor=whimboo][lang=js][lib], URL)


(4 attachments, 2 obsolete attachments)



6 years ago
Our Mozmill l10n tests have been shown that there are some elements which are cut-off due to the localization. The results of the test you can find here:



    Node is cut off by 10 px at the bottom: listitem#en-us. Parent node: listboxbody[rows=6][xbl:inherits=rows,size,minheight]

Comment 1

6 years ago
Hi Henrik,

How would I find where I should correct this?

Comment 2

6 years ago
CC'ing Axel who was helping with this feature. I agree that this information is not that helpful when looking at the plain text results. The screenshots should be more helpful but those are not available yet. I'm working at least at the next step here: https://github.com/mozilla/mozmill-ci/issues/108

I could try to find those screenshots for this locale.

Comment 3

6 years ago
Created attachment 631888 [details]

That's also the same listbox for the ve locale.

Comment 4

6 years ago
The screenshots do really help.

Not important for this locale as it was a cleanup on my side that triggered a build.  But thought I'd anticipate this for the other teams that are live and active.

Some OCR on that could make for some searchable strings to get us to the right page.  I'm dreaming ;)

Comment 5

6 years ago
Increasing <!ENTITY window.width "30em"> in tn/browser/chrome/browser/preferences/languages.dtd will help.

Sadly, I don't know where the height of that dialog is coming from.

Comment 6

6 years ago
I actually think this one is a false positive.  The list is just longer for tn then normal I think.

Comment 7

6 years ago
Looks like that we should exclude list/tree items in that case?

Comment 8

6 years ago
So I agree. I can reproduce it with an en-US locale by just adding more languages to the accepted languages list.

So it looks like that we should check first if the direction is scrollable, if not we can perform those tests. Otherwise we should return early. Axel, does that sound ok to you?
Assignee: dwayne → nobody
Component: tn / Tswana → Mozmill Tests
Product: Mozilla Localizations → Mozilla QA
QA Contact: dwayne → mozmill-tests
Summary: Elements cut-off in preferences dialog for tn locale → Mozmill tests for cut-off elements should not report failure for scrollable direction


6 years ago
Whiteboard: [mozmill-l10n] → [mozmill-l10n][lib]


6 years ago
Whiteboard: [mozmill-l10n][lib] → [mozmill-l10n][mentor=whimboo][lang=js][lib]


5 years ago
Blocks: 763470


5 years ago
Assignee: nobody → andrei.eftimie

Comment 9

5 years ago
Created attachment 740220 [details] [diff] [review]
Patch v1

I've removed richlistbox items with a vertical orient, which targets only the failed test.
We get a green testrun on OSX and Windows:

I've separately attached an unskip patch because we still fail on Linux (seems to be some other cause):

Apply both patches to be able to run the test.
The unskip patch should not land yet, as we should probably resolve the Linux problems first.
Attachment #740220 - Flags: review?(andreea.matei)

Comment 10

5 years ago
Created attachment 740221 [details] [diff] [review]

Comment 11

5 years ago
Comment on attachment 740220 [details] [diff] [review]
Patch v1

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

::: lib/localization.js
@@ +99,5 @@
>    var parent = childBox.parentBox;
> +  // skip for vertical richlistbox items
> +  if (childBox.element.nodeName === "richlistbox" &&
> +      childBox.element.orient === "vertical") {

There are various kinds of elements which have scrollbars. So we cannot exclude richlistbox only. Also elements can have horizontal scrollbars. So this approach will not work.

scrollHeight or something similar might help:
Attachment #740220 - Flags: review?(andreea.matei) → review-

Comment 12

5 years ago
Created attachment 774533 [details] [diff] [review]
patch v3

I've fixed the issue by using offsetWidth and offsetHeight instead of width and height.

The l10n crop test passes on all platforms:
OSX: http://mozmill-crowd.blargon7.com/#/l10n/report/5aa1ca5e7015e3740d269dc9472931a6
Linux: http://mozmill-crowd.blargon7.com/#/l10n/report/5aa1ca5e7015e3740d269dc947298fcd
Windows: http://mozmill-crowd.blargon7.com/#/l10n/report/5aa1ca5e7015e3740d269dc94729c965
Attachment #740220 - Attachment is obsolete: true
Attachment #740221 - Attachment is obsolete: true
Attachment #774533 - Flags: review?(andreea.matei)


5 years ago
Blocks: 763638
Comment on attachment 774533 [details] [diff] [review]
patch v3

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

http://hg.mozilla.org/qa/mozmill-tests/rev/14d962b72438 (default)

We will need backport on this in order to have the l10n tests fixed.
Attachment #774533 - Flags: review?(andreea.matei) → review+
status-firefox22: --- → affected
status-firefox23: --- → affected
status-firefox24: --- → affected
status-firefox25: --- → fixed
status-firefox-esr17: --- → affected

Comment 14

5 years ago
I wouldn't backport this for all branches but aurora only. Reason is that we only run l10n tests on that branch. But if it turns out that the patch applies cleanly, I'm open to even backport it further. Lets get this into aurora, so that we can re-enable the test.

Btw. this patch should not have been re-enabled the test. I'm fairly sure we have a bug open which disabled the test. Also the manifest file hasn't been taken care of.
status-firefox22: affected → wontfix
status-firefox23: affected → wontfix
status-firefox25: fixed → affected
status-firefox-esr17: affected → wontfix

Comment 15

5 years ago
Created attachment 775666 [details] [diff] [review]
reskip followup

reskip followup patch

As discussed we'll unskip the test in bug 741301
Attachment #775666 - Flags: review?(andreea.matei)
Comment on attachment 775666 [details] [diff] [review]
reskip followup

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

Sorry, my mistake here. Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e20b71e03c74 (default)
Attachment #775666 - Flags: review?(andreea.matei) → review+

Comment 17

5 years ago
Created attachment 775672 [details] [diff] [review]
patch v3.1

Applies cleanly an all branches. 
(contains fix, test still skipped)
Attachment #775672 - Flags: review?(andreea.matei)

Comment 18

5 years ago
Ok, so lets get this backported to aurora immediately, and the test enabled. Andrei, I assume you have tested the patch with various locales also with a RTL one?
status-firefox25: affected → fixed

Comment 19

5 years ago
Yep, works fine on .ar which is a RTL language
Comment on attachment 775672 [details] [diff] [review]
patch v3.1

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

Great. Andrei tested all branches and it applied cleanly so here we have:

http://hg.mozilla.org/qa/mozmill-tests/rev/da282053b9bf (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/81927d0b853b (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/e482db7a858a (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/6d27262378b1 (esr17)

Lets get the test enabled now.
Attachment #775672 - Flags: review?(andreea.matei) → review+
Last Resolved: 5 years ago
status-firefox22: wontfix → fixed
status-firefox23: wontfix → fixed
status-firefox24: affected → fixed
status-firefox-esr17: wontfix → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.