Closed
Bug 1050253
Opened 10 years ago
Closed 10 years ago
[ff, fa] Test failure "node.localName is undefined" in testSafeBrowsingNotificationBar.js
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(firefox32 fixed, firefox33 fixed, firefox34 fixed, firefox35 fixed, firefox-esr31 fixed)
People
(Reporter: AndreeaMatei, Assigned: cosmin-malutan)
References
()
Details
(Whiteboard: [mozmill-test-failure][sprint])
Attachments
(2 files)
This fails with Beta ff and fa locales, all platforms: http://mozmill-sandbox.blargon7.com/#/remote/report/ff9854eb4c07409123869856274b0d34 http://mozmill-sandbox.blargon7.com/#/remote/report/ff9854eb4c07409123869856274a44ce I can reproduce this on my Linux, with ff locale, only by running the test.
Comment 1•10 years ago
|
||
Which element is that about? May be good to also add this to the summary.
Reporter | ||
Comment 2•10 years ago
|
||
Yeah, I wanted to add the line in a follow up comment, was still tracking it cause it doesn't seem that obvious in the 'show more' lines. In the test is as this line: http://hg.mozilla.org/qa/mozmill-tests/file/mozilla-beta/firefox/tests/remote/testSecurity/testSafeBrowsingNotificationBar.js#l118 For the button not a forgery in the notification bar. Points in tabs: http://hg.mozilla.org/qa/mozmill-tests/file/mozilla-beta/firefox/lib/tabs.js#l717 Then in mozmill -> mozelement I couldn't find the correlation at those lines, seems here: https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/driver/mozelement.js#L852 https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/driver/mozelement.js#L40 Although in the test I don't see a checkbox mentioned.
Comment 3•10 years ago
|
||
I assume that the panel is not shown, which we try to access here.
Assignee | ||
Comment 4•10 years ago
|
||
Okey I found the issue here. After returning the node, when trying to to instantiate the new mozelemnet it goes through a factory method to check the type of node, for this it tries to get the nodes 'localName' which is undefined. https://github.com/mozilla/mozmill/blob/2.0rc6/mozmill/mozmill/extension/resource/driver/mozelement.js#L847 The reason it's undefined here is because we assume the node is a node but it's an array, because the lookup expression returned two nodes. https://github.com/mozilla/mozmill/blob/2.0rc6/mozmill/mozmill/extension/resource/driver/elementslib.js#L294 That's because we have the same lookup expression for both buttons, they bot have the same access key(see the attached image). I will look why we have the duplicate in the access key, but I honestly think we should also file a bug in mozmill to log this exception better so next time we hit this we would know why it fails, and we won't waste time doing the same investigation as here.
Assignee: nobody → cosmin.malutan
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
Interesting. It indeed sounds like an access key conflict then. Also I was not aware that elementslib is returning arrays. We simply haven't used it so far. Good to know.
Comment 6•10 years ago
|
||
Good find Cosmin. Please file a bug for each locale to get this access key fixed. And file a bug against mozmill to fix the expression to return only one node if there are multiple instances (as we have for the Selector method)
Assignee | ||
Comment 7•10 years ago
|
||
I filed a bug 1053150 for mozmill. I couldn't reproduce the failure for "fa" locale, and it doesn't have a duplicate access key.
Comment 8•10 years ago
|
||
Can we please get this temporarily fixed in our tests. It should not block getting the locale from the blacklist. We can check for an array and take the first element, or the element directly if not an array.
Priority: -- → P1
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #8) > Can we please get this temporarily fixed in our tests. It should not block > getting the locale from the blacklist. We can check for an array and take > the first element, or the element directly if not an array. We can get this permanently fixed if we switch to label instead of access key terms of element locator. Regarding the issue in mozmill we can't handle it in a easy way in mozmill-tests (we will have to switch to using node collector with multiple query for returning a single element) Henrik what do you think about this fix? I think that using lable is much more safer because it will always be different, and also the change will be small and clean. http://mozmill-crowd.blargon7.com/#/remote/report/53ec3148fca67aea1b1c3528f9244505 http://mozmill-crowd.blargon7.com/#/remote/report/53ec3148fca67aea1b1c3528f9247419
Attachment #8498069 -
Flags: review?(andrei.eftimie)
Attachment #8498069 -
Flags: review?(andreea.matei)
Attachment #8498069 -
Flags: feedback?(hskupin)
Comment 10•10 years ago
|
||
Comment on attachment 8498069 [details] [diff] [review] 1050253.patch Review of attachment 8498069 [details] [diff] [review]: ----------------------------------------------------------------- Looks perfect to me.
Attachment #8498069 -
Flags: feedback?(hskupin) → feedback+
Comment 11•10 years ago
|
||
Nightly and Aurora have already been fixed. We have 2 options here: - land the fix only for Beta, Release, ESR31. (hopefully train merges will run smoothly) - land the fix across branches to maintain parity (we'll lose our ability to catch future regressions in regard to access keys)
status-firefox33:
--- → affected
status-firefox34:
--- → fixed
status-firefox35:
--- → fixed
status-firefox-esr31:
--- → affected
Comment 12•10 years ago
|
||
Comment on attachment 8498069 [details] [diff] [review] 1050253.patch Review of attachment 8498069 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me. Spot-checked a few locales and we don't seem to have any issues with the label.
Attachment #8498069 -
Flags: review?(andrei.eftimie)
Attachment #8498069 -
Flags: review?(andreea.matei)
Attachment #8498069 -
Flags: review+
Comment 13•10 years ago
|
||
remote: https://hg.mozilla.org/qa/mozmill-tests/rev/ebb8b498f87a (default) remote: https://hg.mozilla.org/qa/mozmill-tests/rev/9ca9049c4d70 (mozilla-aurora) remote: https://hg.mozilla.org/qa/mozmill-tests/rev/11976fc5de5d (mozilla-beta) remote: https://hg.mozilla.org/qa/mozmill-tests/rev/a110ad907a22 (mozilla-release) remote: https://hg.mozilla.org/qa/mozmill-tests/rev/f1763a220c1d (mozilla-esr31)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
Forgot to mention, as discussed on IRC, we decided to land it across branches for parity. We are working on different tests specifically aimed at Access Keys. These kind of regression will be caught by them in the future.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][sprint]
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•