Right now, the Permissions section is only visible when there are some non-default permissions for the site. For Bug 1188355, we should make it visible all the time, and then have the following text visible if there are no non-default preferences: "You have not granted this page any special permissions."

Comment on attachment 8657064 [details] [diff] [review] 0001-Bug-1193004-Always-show-permissions-section-in-the-C.patch Review of attachment 8657064 [details] [diff] [review]: ----------------------------------------------------------------- The browser_permissions.js is going to need to be updated since it will fail with this patch applied. Can you modify that test to check is_hidden on the description element instead of permissionsContainer? ::: browser/locales/en-US/chrome/browser/browser.dtd @@ +729,5 @@ > > <!ENTITY identity.moreInfoLinkText2 "More Information"> > > <!ENTITY identity.permissions "Permissions"> > +<!ENTITY identity.permissionsEmpty "You have not granted this page any special permissions."> Ash, is this string still correct with the new design work?

(In reply to Brian Grinstead [:bgrins] from comment #2) > Ash, is this string still correct with the new design work? Also, do we still want the 'always shown' permissions section in the main view for 43 or should that wait for 44 along with the subview changes?

(In reply to Brian Grinstead [:bgrins] from comment #3) > (In reply to Brian Grinstead [:bgrins] from comment #2) > > Ash, is this string still correct with the new design work? > > Also, do we still want the 'always shown' permissions section in the main > view for 43 or should that wait for 44 along with the subview changes? The main view can show as long as it matches existing behavior of the current release version users have (https://www.dropbox.com/s/c2telu40akl3p10/Screenshot%202015-09-03%2017.56.43.png?dl=0) in terms of functionality until we update it with the subpanel in 44. For the string - we have currently: 'Requested by this site.' - it doesnt change whether you have something showing or not below it. For now, we can use 'No permissions have been requested.'

(In reply to agrigas from comment #4) > For now, we can use 'No permissions have been requested.' This isn't accurate either. A site could have requested a permission with the user ignoring the request or granting / denying it without persisting that choice.

Didn't change the string, the current version seems to make the most sense.

Permissions are generally site-specific, not page-specific, and the string should probably reflect that.

(In reply to Dão Gottwald [:dao] from comment #5) > (In reply to agrigas from comment #4) > > For now, we can use 'No permissions have been requested.' > > This isn't accurate either. A site could have requested a permission with > the user ignoring the request or granting / denying it without persisting > that choice. Dao - we're changing the behavior or allowing and blocking notifications. The new design will not have a 'dismiss' action so a user has to decide to allow or block temporarily or permanently for the requested permission. I agree we should use 'site' - that is what we have in our latest design. We will have a second copy pass at all this work so what we decide now is placeholder. https://www.dropbox.com/s/fevthj7yelzre27/Screenshot%202015-09-09%2010.20.19.png?dl=0

Comment on attachment 8658715 [details] [diff] [review] 0001-Bug-1193004-Always-show-permissions-section-in-the-C.patch, v2 Review of attachment 8658715 [details] [diff] [review]: ----------------------------------------------------------------- r=me, but please wait to land this until after Bug 1203280, since otherwise there will be some complicated rebasing needed. I will attach a version of this patch rebased on top of 1203280 for convenience ::: browser/base/content/test/general/browser_permissions.js @@ +40,5 @@ > > gIdentityHandler.setPermission("install", SitePermissions.getDefault("install")); > > gIdentityHandler._identityBox.click(); > + ok(!is_hidden(emptyLabel), "The container is hidden"); "List of permissions is empty"

Rebased version of this patch on top of patches in bug 1203280. Also updated the assertion message as I noted in the review

Comment on attachment 8658715 [details] [diff] [review] 0001-Bug-1193004-Always-show-permissions-section-in-the-C.patch, v2 Review of attachment 8658715 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/locales/en-US/chrome/browser/browser.dtd @@ +729,5 @@ > > <!ENTITY identity.moreInfoLinkText2 "More Information"> > > <!ENTITY identity.permissions "Permissions"> > +<!ENTITY identity.permissionsEmpty "You have not granted this page any special permissions."> I guess change this to 'site' as per Comment 8

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4664200&repo=fx-team

Backout: https://hg.mozilla.org/integration/fx-team/rev/9e23f3276571

Not positive, but you may need to use `gBrowser.ownerDocument` instead of `document` here: https://hg.mozilla.org/integration/fx-team/rev/98f6e793e383#l2.14

I can reproduce this locally when I run the test with --run-until-failure. It opens the control center and I can see the <description>, yet document.querySelector() isn't able to find it. Even assigning an ID and using getElementById() doesn't help.

Found something that seems to work.

I reproduced this bug on Firefox Nightly Version 42.0a1 It's fixed and verified on Latest Nightly Build ID 20150917030229 User Agent Mozilla/5.0 (Windows NT 6.3; rv:43.0) Gecko/20100101 Firefox/43.0 Tested OS -- Windows8.1 32bit

Successfully reproduce this bug in Nightly 42.0a1 (2015-08-10) on Linux x64 This Bug is now verified as fixed on Latest Nightly 43.0a1 (2015-09-17) Build ID: 20150917030229 User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 As it is also verified on Windows (Comment 20), Marking it as verified!