Closed Bug 1288690 Opened 8 years ago Closed 8 years ago

Add tests for XUL bindings

Categories

(L20n :: JS Library, defect)

Product:
L20n
Component:
JS Library
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: stas, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

Bug 1288690 - Add tests for XUL bindings
58 bytes, text/x-review-board-request
Pike
: review+
Details 
Add tests for the XUL bindings that can run in mozilla-central.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Attachment #8808838 - Flags: review?(stas)
Attachment #8808838 - Flags: review?(stas)
Comment on attachment 8808838 [details]
Bug 1288690 - Add tests for XUL bindings

https://reviewboard.mozilla.org/r/91572/#review91406

::: toolkit/content/tests/chrome/test_l20n.xul:25
(Diff revision 3)
> +
> +  setTimeout(() => {
> +    const item = document.getElementById('file-menu');
> +    ok(item.hasAttribute('label'), "passed");
> +    SimpleTest.finish();
> +  }, 500);

Can you explain to me the consequences of waiting 500ms here?  In particular:

 - will it cause the suite to run .5s longer?
 - can it cause intermittent failures if l20n's logic happens to take longer than 500ms?

::: toolkit/content/tests/chrome/test_l20n.xul:32
(Diff revision 3)
> +  </script>
> +
> +  <menuitem id="file-menu" data-l10n-id="file-menu" />
> +  <body xmlns="http://www.w3.org/1999/xhtml">
> +  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id="
> +     target="_blank">Mozilla Bug </a>

Either remove this or fill in the bug number, please.  Same in line 5 and line 7.
The timeout is showing our lack of "yeah, do stuff with the localized UI" that matjaz is also missing for pontoon.
yes, I'm happy to update the test once we have document.l10n.ready.
Comment on attachment 8808838 [details]
Bug 1288690 - Add tests for XUL bindings

https://reviewboard.mozilla.org/r/91572/#review91406

> Can you explain to me the consequences of waiting 500ms here?  In particular:
> 
>  - will it cause the suite to run .5s longer?
>  - can it cause intermittent failures if l20n's logic happens to take longer than 500ms?

Yes, at this point we don't seem to have a better way to verify that the translations have been applied.

Once we have document.l10n.ready promise that gets resolved after translateRoot completes, I'll switch to it.
Since there's no rush to get this one test in, can we first fix bug 1316083, then add `document.l10n.ready` in a new bug, and then land this bug?

I don't see much benefit in landing this right now knowing that we'll want to fix it anyways.
(In reply to Staś Małolepszy :stas from comment #9)
> Since there's no rush to get this one test in, can we first fix bug 1316083,
> then add `document.l10n.ready` in a new bug, and then land this bug?

Forgot that Matjaz already filed bug 1315811 for the second item on this list.
Depends on: 1315811
:stas, this is ready for your review now.
Attachment #8808838 - Flags: review?(stas) → review?(l10n)
Comment on attachment 8808838 [details]
Bug 1288690 - Add tests for XUL bindings

https://reviewboard.mozilla.org/r/91572/#review96512

::: toolkit/content/tests/chrome/test_l20n.xul:10
(Diff revision 5)
> +<window title="L20n.js XUL bindings test"
> +        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
> +  <script type="application/javascript"
> +          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
> +
> +  <link rel="localization" href="/browser/menubar.ftl"/>

Let's see if we can use a data uri here.
Comment on attachment 8808838 [details]
Bug 1288690 - Add tests for XUL bindings

Oh, MozReview...
Attachment #8808838 - Flags: review?(stas) → review?(l10n)
Comment on attachment 8808838 [details]
Bug 1288690 - Add tests for XUL bindings

https://reviewboard.mozilla.org/r/91572/#review97104
Attachment #8808838 - Flags: review?(l10n) → review+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: