1306536 - Do not display origin when displayURI is set to false in PermissionUI.jsm

Status: RESOLVED FIXED

(Firefox :: Site Permissions, defect)

Description

[C.M.Chang[:chunmin]]

Attachment: Do not set displayURI if it's set to false

When displayURI is exactly set to false in PermissionPrompt, we should not display the origin containing the URI's information.

[0] http://searchfox.org/mozilla-central/rev/5bdd2876aeb9dc97dc619ab3e067e86083dad023/browser/modules/PermissionUI.jsm#332

Comment 1

[C.M.Chang[:chunmin]]

Attachment: Bug 1306536 - Don't set displayURI when it's set to false;

Review commit: https://reviewboard.mozilla.org/r/82276/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82276/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8796407 [details]
Bug 1306536 - Don't set displayURI when it's set to false;

https://reviewboard.mozilla.org/r/82276/#review81402

Looks great, thanks! Would be good to have a test for this as well. Can you add one (or modify one of the existing ones) in browser/modules/test/browser_PermissionUI.js ? Thanks!

Comment 3

[C.M.Chang[:chunmin]]

Attachment: Bug 1306536 - Test;

Review commit: https://reviewboard.mozilla.org/r/82956/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82956/

Comment 4

[C.M.Chang[:chunmin]]

Comment on attachment 8796407 [details]
Bug 1306536 - Don't set displayURI when it's set to false;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82276/diff/1-2/

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8797368 [details]
Bug 1306536 - Test;

https://reviewboard.mozilla.org/r/82956/#review81738

Looks great! Thanks for the test!

::: browser/modules/test/browser_PermissionUI.js:161
(Diff revision 1)
>      yield removePromise;
>    });
>  });
>  
>  /**
> + * Tests that if the PermissionPrompt set displayURI to false in popupOptions,

Nit: "set" -> "sets"

::: browser/modules/test/browser_PermissionUI.js:187
(Diff revision 1)
> +      __proto__: PermissionUI.PermissionPromptForRequestPrototype,
> +      request: mockRequest,
> +      notificationID: kTestNotificationID,
> +      message: kTestMessage,
> +      promptActions: [mainAction, secondaryAction],
> +      popupOptions: { displayURI: false, },

Let's break this up over several lines, like:

```JavaScript
popupOptions: {
  displayURI: false,
},
```

Comment 6

[C.M.Chang[:chunmin]]

Comment on attachment 8797368 [details]
Bug 1306536 - Test;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82956/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6cc9dcdf0a7
Don't set displayURI when it's set to false; r=mconley
https://hg.mozilla.org/integration/autoland/rev/2ba7a111b2a7
Test; r=mconley

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a6cc9dcdf0a7
https://hg.mozilla.org/mozilla-central/rev/2ba7a111b2a7