Closed
Bug 944337
Opened 11 years ago
Closed 11 years ago
Test failure "Modal dialog has been found and processed" in /testAddons/testInstallAddonWithEULA.js
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(firefox25 wontfix, firefox26 unaffected, firefox27 affected, firefox28 fixed, firefox29 fixed, firefox-esr17 wontfix, firefox-esr24 unaffected)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox25 | --- | wontfix |
firefox26 | --- | unaffected |
firefox27 | --- | affected |
firefox28 | --- | fixed |
firefox29 | --- | fixed |
firefox-esr17 | --- | wontfix |
firefox-esr24 | --- | unaffected |
People
(Reporter: mario.garbi, Assigned: andrei)
References
(Depends on 1 open bug, )
Details
(Whiteboard: [mozmill-test-failure][needs Mozmill 2.0.3 for final fix])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
andrei
:
checkin+
|
Details | Diff | Splinter Review |
Happened today with Mozmill 2.0 on OSX machines and Firefox 27:
http://mozmill-daily.blargon7.com/#/remote/report/b99421c0f132c68dec1548288a3a7f52
http://mozmill-daily.blargon7.com/#/remote/report/b99421c0f132c68dec1548288a3a8d88
Reporter | ||
Updated•11 years ago
|
status-firefox25:
--- → ?
status-firefox26:
--- → ?
status-firefox27:
--- → affected
status-firefox28:
--- → ?
status-firefox29:
--- → ?
status-firefox-esr17:
--- → ?
status-firefox-esr24:
--- → ?
Comment 1•11 years ago
|
||
I think something is wrong with the add-on we are trying to install from that site. Can you please check that?
Reporter | ||
Comment 2•11 years ago
|
||
Hmm the addon is working ok for me, plus this is not reproducing constantly.
Assignee | ||
Comment 3•11 years ago
|
||
I'll take this
Assignee: nobody → andrei.eftimie
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 4•11 years ago
|
||
This is a very similar (if not the same) issue to bug 939690
It also fails while running the tests:
> restartTests/testAddons_InstallAddonWithoutEULA
> testAddons/testInstallAddonWithEULA.js
with --profile
Assignee | ||
Comment 5•11 years ago
|
||
This is a quick hack.
We fail to either preserve or set addons.mozilla.org as a whitelist entry.
Setting it manually in each test fixes the issue.
We should probably get this in mozmill/mozmill-automation/mozprofile (or where we are setting these settings).
Do we want to get this in to fix the current failures?
(And remove it once we fix the underlying issue)
Attachment #8339952 -
Flags: feedback?(hskupin)
Attachment #8339952 -
Flags: feedback?(dave.hunt)
Attachment #8339952 -
Flags: feedback?(andreea.matei)
Assignee | ||
Comment 6•11 years ago
|
||
Remote testrun:
http://mozmill-crowd.blargon7.com/#/remote/report/b99421c0f132c68dec1548288a4a978e
The failure we still see is bug 944334
Assignee | ||
Comment 7•11 years ago
|
||
Patch for ESR17.
Report:
http://mozmill-crowd.blargon7.com/#/remote/report/b99421c0f132c68dec1548288a4f12bd
Attachment #8339971 -
Flags: review?(andreea.matei)
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8339971 [details] [diff] [review]
esr17_1.patch
Disregard this, was intended for bug 944334
Attachment #8339971 -
Attachment is obsolete: true
Attachment #8339971 -
Flags: review?(andreea.matei)
Comment 9•11 years ago
|
||
Comment on attachment 8339952 [details] [diff] [review]
1.patch
Review of attachment 8339952 [details] [diff] [review]:
-----------------------------------------------------------------
::: firefox/tests/remote/restartTests/testAddons_InstallAddonWithoutEULA/test1.js
@@ +17,4 @@
>
> const INSTALL_DIALOG_DELAY = 250;
> const TIMEOUT_DOWNLOAD = 25000;
> +const XPI_WHITELIST_DOMAIN = "addons.mozilla.org";
this domain should be whitelisted by default. Why is that no longer the case after a couple of restart tests have been run? This clearly looks like a problem in Firefox or a test which removes this entry from the whitelist. We should not workaround but fix the underlying issue.
Attachment #8339952 -
Flags: feedback?(hskupin)
Attachment #8339952 -
Flags: feedback?(dave.hunt)
Attachment #8339952 -
Flags: feedback?(andreea.matei)
Attachment #8339952 -
Flags: feedback-
Assignee | ||
Comment 10•11 years ago
|
||
I agree, I added a quick fix for us to avoid additional failures since we were running 2.0 in production.
Comment 11•11 years ago
|
||
We are back at 1.5.24 so there is no pressure today. But I really want to see it investigated and fixed asap. Lets observe this permission between tests so we know where it is getting reset.
Assignee | ||
Comment 12•11 years ago
|
||
I still have to dig a bit here.
The permissions.sqlite seems to be fine, the problem appears to lie in pref.js
I haven't yet been able to single-out the pref that is causing this.
If I clean the pref.js fine (or certain parts of it) the tests are passing nicely.
Assignee | ||
Comment 13•11 years ago
|
||
*prefs.js file*
Assignee | ||
Comment 14•11 years ago
|
||
We have the following 2 preferences:
> xpinstall.whitelist.add
> xpinstall.whitelist.add.180
Which are both being set to ""
Their default values are:
> xpinstall.whitelist.add : addons.mozilla.org
> xpinstall.whitelist.add.180 : marketplace.firefox.com
I'm trying to figure out *who* sets them and *when* they are set.
Haven't had success searching for the pref name on either mxr or mozmill related repositories.
Assignee | ||
Comment 15•11 years ago
|
||
After a whole lot more digging, the mentioned preferences are set when we try to install the addon from amo:
https://addons.mozilla.org/en-US/firefox/addon/restartless-addon-with-eula/eula/227209
I've tested manually with a clean profile.
When I try to install the addon from mozqa.com
http://mozqa.com/data/firefox/addons/extensions/restartless-eula.xpi
The change of preferences are still visible even if I cancel the install.
In this case I have to accept to install the addon (since it is not from a 'whitelisted' domain).
I'm not sure if this action sets the mentioned preferences.
I'll test with other addons and Firefox versions.
Assignee | ||
Comment 16•11 years ago
|
||
I've tested a new profile on all current Firefox builds.
`xpi.whitelist.add` is left at the default state of `addons.mozilla.org` only on Nightly and Aurora
All Beta, Release and ESR24 builds show this preference user set to empty string "".
This is all on a clean profile.
Weird is that even if this preference appears to be set initially (I've added a long sleep at the begging of the test) and checked both about:config and pref.js from the profile folder. The first run always passes. So there should also be something else that affects us here.
But if I remove only this preference from the prefs.js file, a subsequent run always passes.
Assignee | ||
Comment 17•11 years ago
|
||
Blair do you know how the preference `xpinstall.whitelist.add` should behave?
Is it normal to have them set to empty when trying to install an addon?
Comment 18•11 years ago
|
||
(In reply to Andrei Eftimie from comment #17)
> Blair do you know how the preference `xpinstall.whitelist.add` should behave?
>
> Is it normal to have them set to empty when trying to install an addon?
You should CC or put needinfo on him if you really want his feedback on this question. :)
Flags: needinfo?(bmcbride)
Assignee | ||
Comment 19•11 years ago
|
||
I think I missed to mark the checkbox for the needinfo box.
Thanks for noticing that.
Comment 20•11 years ago
|
||
(In reply to Andrei Eftimie from comment #17)
> Blair do you know how the preference `xpinstall.whitelist.add` should behave?
>
> Is it normal to have them set to empty when trying to install an addon?
Yes, this is expected.
We use the permissions manager to determine whether a site is allowed to install an add-on. These preferences are just a simple and convenient way to get hosts into the permissions manager. If one of these preferences has a non-empty string value, it's imported into the permissions manager the first time a site requests an install*. We set it to an empty string so it isn't imported more than once (for performance reasons).
* Originally this was done at startup. Bug 697314 changed that recently (for performance reasons) so it only happens when we find we need to check the permissions manager, but still only happens once after startup.
Flags: needinfo?(bmcbride)
Assignee | ||
Comment 21•11 years ago
|
||
Attachment #8344533 -
Flags: review?(hskupin)
Attachment #8344533 -
Flags: review?(dave.hunt)
Attachment #8344533 -
Flags: review?(andreea.matei)
Assignee | ||
Comment 22•11 years ago
|
||
Pressed Enter to quickly.
Given that `xpinstall.whitelist.add` works as expected, lets clean up after we install some addons.
This fixes our remote testrun under mozmill 2.0:
http://mozmill-crowd.blargon7.com/#/remote/report/8ff37f5518a39f6af2983cfe4402aa2e
Assignee | ||
Updated•11 years ago
|
Attachment #8339952 -
Attachment is obsolete: true
Comment 23•11 years ago
|
||
Comment on attachment 8344533 [details] [diff] [review]
2.patch
Review of attachment 8344533 [details] [diff] [review]:
-----------------------------------------------------------------
::: firefox/tests/remote/testAddons/testInstallAddonWithEULA.js
@@ +39,5 @@
>
> function teardownModule(aModule) {
> prefs.preferences.clearUserPref(PREF_INSTALL_DIALOG);
> prefs.preferences.clearUserPref(PREF_LAST_CATEGORY);
> + prefs.preferences.clearUserPref(PREF_XPI_WHITELIST);
Well, I'm still not sure why we have to reset this pref, and why addons.mozilla.org is not whitelisted anymore after we install the first add-on. Once added to the permission manager the domain should always be whitelisted.
Attachment #8344533 -
Flags: review?(hskupin)
Attachment #8344533 -
Flags: review?(dave.hunt)
Attachment #8344533 -
Flags: review?(andreea.matei)
Attachment #8344533 -
Flags: review-
Assignee | ||
Comment 24•11 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #23)
> Well, I'm still not sure why we have to reset this pref, and why
> addons.mozilla.org is not whitelisted anymore after we install the first
> add-on. Once added to the permission manager the domain should always be
> whitelisted.
I dug around more, and I see what you mean here.
We do seem to have a problem, after a test has finished running via mozmill the permissions.sqlite is emptied!
Comment 25•11 years ago
|
||
I cannot wait longer for a solution on this bug. Given that it has been stalled for nearly a week I'm glancing to take over the work which has to happen here. Andrei, is a final solution visible for today?
Flags: needinfo?(andrei.eftimie)
Assignee | ||
Comment 26•11 years ago
|
||
Lets recap what I said on IRC.
I don't have another working solution ATM besides clearing the xpinstall pref.
For having a green 2.0 green run, we could go with that and file a followup bug to fix the underlying issue.
I am not sure, but from what I saw we end up cleaning the profile multiple times.
Roughly I think this was it: https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/profile.py#L113
When this is done the permissions.sqlite is nuked, but we remain with the user set prefs for xpinstall.
Flags: needinfo?(andrei.eftimie)
Comment 27•11 years ago
|
||
Ok, that sounds like the problem. Can you please file a bug in mozbase for that problem? If we clean-up the profile we should also clean-up non-default preferences.
Assignee | ||
Comment 28•11 years ago
|
||
Comment on attachment 8344533 [details] [diff] [review]
2.patch
Filed bug 944337 for fixing the underlying issue with permissions vs prefs in mozbase.
I've reissued review flags for this patch.
This will allow us to successfully run these tests with mozmill 2.0
Attachment #8344533 -
Flags: review?(hskupin)
Attachment #8344533 -
Flags: review?(andreea.matei)
Comment 29•11 years ago
|
||
Comment on attachment 8344533 [details] [diff] [review]
2.patch
Review of attachment 8344533 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine. Two nits to fix before we can get this landed. Please do so ASAP.
::: firefox/tests/remote/restartTests/testAddons_InstallAddonWithoutEULA/test1.js
@@ +38,5 @@
> tabs.closeAllTabs(aModule.controller);
> }
>
> function teardownModule(aModule) {
> + prefs.preferences.clearUserPref(PREF_XPI_WHITELIST);
Please add a comment why this is necessary to do.
Attachment #8344533 -
Flags: review?(hskupin)
Attachment #8344533 -
Flags: review?(andreea.matei)
Attachment #8344533 -
Flags: review-
Attachment #8344533 -
Flags: review+
Assignee | ||
Comment 30•11 years ago
|
||
Added a comment.
I wrongly referenced this bug in comment 28.
It should have been bug 951138
Landed it:
http://hg.mozilla.org/qa/mozmill-tests/rev/a4009c80179d (default)
Attachment #8344533 -
Attachment is obsolete: true
Attachment #8348721 -
Flags: checkin+
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 31•11 years ago
|
||
BTW, green testrun on remote with 2.0.2:
http://mozmill-crowd.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01ed3dc37
Assignee | ||
Comment 32•11 years ago
|
||
Fixes the problem on Aurora:
http://mozmill-crowd.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01ed4558b
Transplanted:
http://hg.mozilla.org/qa/mozmill-tests/rev/c3edf041487c (mozilla-aurora)
Unfortunately this patch doesn't resolve the issue on Beta :(
I'll check all remaining branches
Assignee | ||
Comment 33•11 years ago
|
||
Neither Release nor ESR24 are affected:
http://mozmill-crowd.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01ed65827
http://mozmill-crowd.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01ed6fae2
Comment 34•11 years ago
|
||
This works now:
http://mozmill-staging.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01edc7239
So lets get it backported.
Comment 35•11 years ago
|
||
(In reply to Andrei Eftimie from comment #32)
> Unfortunately this patch doesn't resolve the issue on Beta :(
Uh, why not? Did you run it correctly?
(In reply to Andrei Eftimie from comment #33)
> Neither Release nor ESR24 are affected:
So it is a regression in Firefox? If only specific branches are affected the underlying issue cannot be a mozprofile one.
Flags: needinfo?(andrei.eftimie)
Assignee | ||
Comment 36•11 years ago
|
||
Not sure what I saw earlier, but this also fixes the problem on Beta:
http://mozmill-crowd.blargon7.com/#/remote/report/ca1869364f98dd0de71202e01edd548f
Transplanted:
http://hg.mozilla.org/qa/mozmill-tests/rev/643480b4394b (mozilla-beta)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: needinfo?(andrei.eftimie)
Resolution: --- → FIXED
Comment 37•11 years ago
|
||
Please do not close this bug until there are still outstanding questions about fixes on the remaining branches!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 38•11 years ago
|
||
We do not have the affected tests on mozilla-release nor on mozilla-esr24.
I see the same failures on both branches if I replicate the conditions.
So we are good.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86_64 → All
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][needs Mozmill 2.0.3 for final fix]
Comment 39•11 years ago
|
||
So it only happens when more than a test installing addons from addons.mozilla.org. I would suggest we leave this bug open until we got the final fix. As of now we only landed a workaround.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 40•11 years ago
|
||
The test was removed in bug 962514.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
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
•