856638 - Allow passing PopupNotification options to _showPrompt in nsBrowserGlue.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Ed Lee :Mardak]

I've got a notification that would like to add in options, but it probably would be cleaner to have my _promptInterests pass in options instead of having _showPrompt handle that logic.

Comment 1

[Ed Lee :Mardak]

Attachment: v1

Comment 2

[Justin Dolske [:Dolske]]

Comment on attachment 731890 [details] [diff] [review]
v1

Review of attachment 731890 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the following.

::: browser/components/nsBrowserGlue.js
@@ +1641,3 @@
>     */
>    _showPrompt: function CPP_showPrompt(aRequest, aMessage, aPermission, aActions,
> +                                       aNotificationId, aAnchorId, aOptions) {

Hmm, I think we'd be better off simply fixing up the 3 callers of _showPrompt to explicitly pass one in, even if null.

I should have just done this in last week's pointerLock patch, this seems so obvious in hindsight.

@@ +1706,5 @@
> +              aRequest.allow();
> +            }
> +          }
> +        },
> +      };

Might as well just move this whole block into _promptPointerLock().

Comment 3

[Ed Lee :Mardak]

Attachment: for checkin

Comment 4

[Ed Lee :Mardak]

(In reply to Justin Dolske [:Dolske] from comment #2)
> Hmm, I think we'd be better off simply fixing up the 3 callers of
> _showPrompt to explicitly pass one in, even if null.
Updated the callers to pass in null and fixed the comment block to not be [optional].

> Might as well just move this whole block into _promptPointerLock().
As discussed on IRC, this isn't a simple move as |eventCallback| relies on a couple names defined in _showPrompt: |browser| and |onFullScreen|, which depends on popup that only gets set after passing in options.

Comment 5

[Ed Lee :Mardak]

https://hg.mozilla.org/integration/mozilla-inbound/rev/075ef2e03d2f
 Add an options parameter and use that variable for the existing pointerLock options. Update existing callers to pass in null.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/075ef2e03d2f