Decouple Permissions Panel from Site Information Panel
Categories
(Firefox :: Site Permissions, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox87 | --- | fixed |
People
(Reporter: epang, Assigned: pbz)
References
(Blocks 3 open bugs)
Details
User Story
Attachments
(3 files)
Decouple Permissions and Site Information Panel.png
With ETP in its own panel and the removal of the 'i' icon, permissions required a bit of rethinking.
Link to the page in the spec
https://mozilla.invisionapp.com/share/VMUV7OL8EUA#/392611065_Decouple_Permissions_And_Site_Information_Panel
This spec covers separating the permission and site information panels as well as other updates to the site information panel.
Link to the meta bug
https://bugzilla.mozilla.org/show_bug.cgi?id=1586946
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 1•3 years ago
|
||
|
|
Assignee | |
Comment 2•3 years ago
|
||
Depends on D99892
Pushed by pzuhlcke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/61304ccbaada Moved permission list from site identity to separate permission panel. r=johannh https://hg.mozilla.org/integration/autoland/rev/00c45a405129 Updated tests for decoupled permissions panel. r=johannh
|
||
Comment 4•3 years ago
|
||
Backed out 2 changesets (bug 1596897) for mochitest failures at browser_setIgnoreCertificateErrors.js.
https://hg.mozilla.org/integration/autoland/rev/88e18d323f43ee04d39e203a2db6e360428b1d16
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=00c45a405129795acbec2944792386dad3f63c26&selectedTaskRun=Vqghr2r7RCi3DkR-0Dqbbw.0
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=328114247&repo=autoland&lineNumber=8229
|
|
Assignee | |
Updated•3 years ago
|
Pushed by pzuhlcke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a84b667007e2 Moved permission list from site identity to separate permission panel. r=johannh https://hg.mozilla.org/integration/autoland/rev/635972f169e7 Updated tests for decoupled permissions panel. r=johannh,remote-protocol-reviewers
|
||
Comment 6•3 years ago
|
||
Backed out for bc failures on browser_search_discovery.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/059539aeb4852ef5b7f37f578ac510dffa1056fe
Log link: https://treeherder.mozilla.org/logviewer?job_id=328268168&repo=autoland&lineNumber=2612
|
|
||
Comment 7•3 years ago
|
||
|
|
Assignee | |
Comment 8•3 years ago
|
||
Thanks!
I don't think any of these test failures are related to my patch.
Focused menu item has correct title - Got "", expected "Two": Bug 1638500
ok is not defined: Bug 1673742.
|
||
Comment 9•3 years ago
|
||
Note that this patch is tripping screenshot alerts; it looks like the ControlCenter.jsm entrypoint was updated to open the identity panel, but this means we lose coverage of the permissions side of things. Is it worth filing a follow-up to re-add coverage for that in some way?
|
|
||
Comment 10•3 years ago
|
||
|
|
Assignee | |
Comment 11•3 years ago
|
||
(In reply to :Gijs (he/him) from comment #9)
Note that this patch is tripping screenshot alerts; it looks like the ControlCenter.jsm entrypoint was updated to open the identity panel, but this means we lose coverage of the permissions side of things. Is it worth filing a follow-up to re-add coverage for that in some way?
Thanks for pointing that out! Yes, I think we should maintain coverage for both panels. I'm not very familiar with mozscreenshots yet. Can you point me to documentation for this / how I can run these tests? Do we file this in the mozscreenshots component?
(In reply to Narcis Beleuzu [:NarcisB] from comment #10)
Thanks! What do you mean by "backfill"? From your link, I can see that the test is failing more frequently with my patches applied. I don't see any direct correlation with my changes though. Perhaps we should consider disabling it, given that it fails quite frequently on this configuration.
|
|
||
Comment 12•3 years ago
|
||
(In reply to Paul Zühlcke [:pbz] from comment #11)
(In reply to :Gijs (he/him) from comment #9)
Note that this patch is tripping screenshot alerts; it looks like the ControlCenter.jsm entrypoint was updated to open the identity panel, but this means we lose coverage of the permissions side of things. Is it worth filing a follow-up to re-add coverage for that in some way?
Thanks for pointing that out! Yes, I think we should maintain coverage for both panels. I'm not very familiar with mozscreenshots yet. Can you point me to documentation for this / how I can run these tests? Do we file this in the mozscreenshots component?
Yes, I think https://developer.mozilla.org/en-US/docs/Mozilla/QA/Browser_screenshots , particularly https://developer.mozilla.org/en-US/docs/Mozilla/QA/Browser_screenshots#Try_pushes, covers all of these? But I am also not intimately familiar with the implementation, I have to admit. :-)
|
|
||
Comment 13•3 years ago
|
||
Hi Paul!
In order to find the culprit of this fail we did some retriggers and backfills.
As I can see, starting with this push, those tests started to fail. Bellow, all retriggs are green
|
|
Assignee | |
Comment 14•3 years ago
|
||
Sorry, I'm just getting back to this now.
Here is a try run of this testsuite/ platform on central:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=Mq1P9uveQySULO5TFoVEbQ.0&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=5e4a7d491693fc0dd5d4ec5884d804a2bcece563
And here is one with the patches from this bug applied:
https://treeherder.mozilla.org/jobs?repo=try&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=3a9402fc45a4f7321552796a92d0c66c51e7c5a5&selectedTaskRun=RkQl2YsNSlejgDu_mozoqQ.0
As you can see it fails for both revisions. It's possible that the timing of this testsuite changed and therefore we get more intermittent failures of these tests.
Here is my suggestion: As long as those two tests don't perma-fail we land the patch. Then we can observe the tests on central and see if they need to be fixed or potentially disabled.
Does that work for you?
The alternative is to disable these tests beforehand. I'd like to avoid loosing test coverage on macOS though.
|
|
||
Comment 15•3 years ago
|
||
Sure, we can land the patches on autoland and if it there wont be any perma failures will merge it to central and will see whats next.
Please notify the sheriffs when you will land them so they will be aware of it
|
||
Comment 16•3 years ago
|
||
Pushed by pzuhlcke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a674c136b1cd Moved permission list from site identity to separate permission panel. r=johannh https://hg.mozilla.org/integration/autoland/rev/a044b2b11f16 Updated tests for decoupled permissions panel. r=johannh,remote-protocol-reviewers
|
||
Comment 17•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a674c136b1cd
https://hg.mozilla.org/mozilla-central/rev/a044b2b11f16
Description
•