Last Comment Bug 600229 - TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | Test timed out
: TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/dataman...
Status: RESOLVED FIXED
[cc-orange]
:
Product: SeaMonkey
Classification: Client Software
Component: Passwords & Permissions (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: seamonkey2.1b1
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on:
Blocks: SmTestFail DataManager
  Show dependency treegraph
 
Reported: 2010-09-28 09:46 PDT by Bruno 'Aqualon' Escherl
Modified: 2012-11-06 06:20 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
export permLabel differently (5.94 KB, patch)
2010-09-28 12:07 PDT, Robert Kaiser
stefanh: review+
Details | Diff | Splinter Review
Proposed patch (9.07 KB, patch)
2010-09-30 12:29 PDT, neil@parkwaycc.co.uk
stefanh: review+
Details | Diff | Splinter Review

Description Bruno 'Aqualon' Escherl 2010-09-28 09:46:28 PDT
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) 2010-09-28 10:13:58 PDT
(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 2010-09-28 10:34:25 PDT
(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 2010-09-28 10:36:29 PDT
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] 2010-09-28 11:19:26 PDT
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 2010-09-28 12:07:46 PDT
Created attachment 479125 [details] [diff] [review]
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] 2010-09-28 12:32:16 PDT
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 2010-09-28 12:41:20 PDT
(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 2010-09-29 16:58:42 PDT
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 neil@parkwaycc.co.uk 2010-09-30 02:41:30 PDT
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 2010-09-30 03:04:22 PDT
(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 neil@parkwaycc.co.uk 2010-09-30 05:29:37 PDT
(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 2010-09-30 06:07:21 PDT
(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 neil@parkwaycc.co.uk 2010-09-30 12:29:57 PDT
Created attachment 479863 [details] [diff] [review]
Proposed patch
Comment 14 neil@parkwaycc.co.uk 2010-09-30 13:45:26 PDT
Pushed changeset 660620295ae1 to comm-central.
Comment 15 Philip Chee 2012-11-06 06:20:35 PST
> Pushed changeset 660620295ae1 to comm-central.
Linkify comm-central changeset 660620295ae1

Note You need to log in before you can comment on or make changes to this bug.