600229 - TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | Test timed out

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Bruno 'Aqualon' Escherl]

Seems to be mac-only:

http://brasstacks.mozilla.com/topfails/test/SeaMonkey?name=chrome://mochikit/content/browser/suite/common/dataman/tests/browser_dataman_basics.js

The only failure I see atm is
*** Failed to get string perm.test.label in bundle: chrome://communicator/locale/dataman/dataman.properties

But this also occurs on Linux, so probably not related to the timeout.

Comment 1

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)

> *** Failed to get string perm.test.label in bundle:
> chrome://communicator/locale/dataman/dataman.properties

http://mxr.mozilla.org/comm-central/source/suite/locales/en-US/chrome/common/dataman/dataman.properties
{
50 # permissions
51 perm.allowXULXBL.label=Use XUL/XBL Markup
52 perm.cookie.label=Set Cookies
53 perm.geo.label=Share Location
54 perm.image.label=Load Images
55 perm.install.label=Install Add-ons
56 perm.password.label=Save Passwords
57 perm.popup.label=Open Popup Windows
}

http://mxr.mozilla.org/comm-central/source/suite/common/dataman/tests/browser_dataman_basics.js
{
296   Services.perms.add(Services.io.newURI("http://test.getpersonas.com/", null, null),
297                      "test", Services.perms.DENY_ACTION);
}
I assume this is the code which tries to get that property.
Fwiw, there also seems to be no 'password' check in that block: I don't know whether there should be one.

Comment 2

[Robert Kaiser]

(In reply to comment #0)
> *** Failed to get string perm.test.label in bundle:
> chrome://communicator/locale/dataman/dataman.properties

Completely unrelated and not an error at all, but expected and caught with a try/catch.

Comment 3

[Robert Kaiser]

Also, comment #1 is not related to the Mac failure at all.

Stefan promised me to look into what's happening on a Mac when running this test.

Thanks for filing, I have seen this from the first test run on Mac on and it's been on my mind since, but I don't know what could be going on there, I really can't move forward without any Mac person looking into it. Stefan said that Data Manager appears to be working basically for him, but I'm waiting on a word about running this test.

Comment 4

[Stefan [:stefanh]]

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | Found a tab after previous test timed out: about:blank

I'm wondering about this one - why do we open a new tab before we open data manager?

Comment 5

[Robert Kaiser]

Attachment: export permLabel differently

OK, so using the .labelElement property thought for screen readers didn't work out that well in the test, as Mac doesn't have that one.

This patch sets a property for the label element manually, so that we can use it in the test (and use it in the code elsewhere as well).

Comment 6

[Stefan [:stefanh]]

Comment on attachment 479125 [details] [diff] [review]
export permLabel differently

Tests now passes fine.

+      <property name="_permLabel

Why is this private?

Comment 7

[Robert Kaiser]

(In reply to comment #6)
> Comment on attachment 479125 [details] [diff] [review]
> export permLabel differently
> 
> Tests now passes fine.

Cool!

> +      <property name="_permLabel
> 
> Why is this private?

Because I thought it would be a good sign to others to say "please don't use this externally". I'm not fond of marking it private, though, let's see what Ian thinks.

Comment 8

[Robert Kaiser]

Comment on attachment 479125 [details] [diff] [review]
export permLabel differently

Switching to Neil instead as apparently Ian doesn't have much time atm.

Comment 9

[[e-mail]]

Comment on attachment 479125 [details] [diff] [review]
export permLabel differently

>+      <property name="_permLabel" readonly="true"
>+                onget="return document.getAnonymousElementByAttribute(this,
>+                                                                      'anonid',
>+                                                                      'permLabel');"/>
It's annoying we have to create this just so tests can use it :-( Can you not add a helper function to the test instead?

>-        is(perm.labelElement.value, "Use XUL/XBL Markup",
>+        is(perm._permLabel.value, "Use XUL/XBL Markup",
Right, labelElement is only used by the access key code. Silly Mac ;-)

Comment 10

[Robert Kaiser]

(In reply to comment #9)
> It's annoying we have to create this just so tests can use it :-( Can you not
> add a helper function to the test instead?

Well, at least we have a user internally in the binding as well. And I'd really like not use use getAnonymousElementByAttribute() outside XBL code, like in a test. If you insist, I can probably do that, but I'd like to avoid it.

> Right, labelElement is only used by the access key code. Silly Mac ;-)

Well, it just seems we don't implement ARIA accessibility stuff there - or at least not the same way as everywhere else. :(

Comment 11

[[e-mail]]

(In reply to comment #10)
> (In reply to comment #9)
> > It's annoying we have to create this just so tests can use it :-( Can you not
> > add a helper function to the test instead?
> Well, at least we have a user internally in the binding as well. And I'd really
> like not use use getAnonymousElementByAttribute() outside XBL code, like in a
> test. If you insist, I can probably do that, but I'd like to avoid it.
New idea: we already use xbl:inherits for the hostLabel; why not use it for the permLabel too? Then it's just a getAttribute call in the test.

> > Right, labelElement is only used by the access key code. Silly Mac ;-)
> Well, it just seems we don't implement ARIA accessibility stuff there - or at
> least not the same way as everywhere else. :(
I meant access key, not accessibility - we don't underline them on the Mac.

Comment 12

[Robert Kaiser]

(In reply to comment #11)
> New idea: we already use xbl:inherits for the hostLabel; why not use it for the
> permLabel too? Then it's just a getAttribute call in the test.

Interesting idea, but I doubt I'll come around to it today, which means I might need to leave this test failing for a few weeks.

Comment 13

[[e-mail]]

Attachment: Proposed patch

Comment 14

[[e-mail]]

Pushed changeset 660620295ae1 to comm-central.

Comment 15

[Philip Chee]

> Pushed changeset 660620295ae1 to comm-central.
Linkify comm-central changeset 660620295ae1