778392 - Use handleEvent() for BrowserOnClick

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Use handleEvent() for BrowserOnClick().
If we use handleEvent(), BrowserOnClick will be clearly.

Comment 1

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch

Comment 2

[Josh Matthews [:jdm]]

Comment on attachment 646826 [details] [diff] [review]
patch

I am not a browser peer, so I'm going to randomly select Felipe.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 646826 [details] [diff] [review]
patch

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

Thanks Ohzeki, this is some great clean-up. If you could please update the patch with the style nits below fixed (and the `safebrowsing` change) I'll land it.

::: browser/base/content/browser.js
@@ +2528,5 @@
>            Components.utils.reportError("Couldn't get ssl_override pref: " + e);
>          }
>  
>          window.openDialog('chrome://pippki/content/exceptionDialog.xul',
> +            '','chrome,centerscreen,modal', params);

this change (here and elsewhere) to the indentation style is incorrect. Multiple line of parameters should all be indented past the opening parens, like it was before

@@ +2600,5 @@
> +        Ci.nsIPermissionManager.EXPIRE_SESSION);
> +
> +    let buttons = [{
> +      label: gNavigatorBundle.getString("safebrowsing.getMeOutOfHereButton.label"),
> +        accessKey: gNavigatorBundle.getString("safebrowsing.getMeOutOfHereButton.accessKey"),

broken indent

@@ +2611,5 @@
> +      buttons[1] = {
> +        label: gNavigatorBundle.getString("safebrowsing.notAnAttackButton.label"),
> +        accessKey: gNavigatorBundle.getString("safebrowsing.notAnAttackButton.accessKey"),
> +        callback: function() {
> +          openUILinkIn(safebrowsing.getReportURL('MalwareError'), 'tab');

note: `safebrowsing` seem to have changed recently to become gSafeBrowsing

(changed in http://hg.mozilla.org/mozilla-central/rev/3b312c31d1cd)

@@ +2757,5 @@
>    _printPreviewTab: null,
>    _tabBeforePrintPreview: null,
>  
>    getPrintPreviewBrowser: function () {
> +    if (!this._printPreviewTab)  {

unrelated change

@@ +6113,5 @@
>          warnAboutClosingTabs = true;
> +      // If the current window is not in private browsing mode we don't need to
> +      // look for other pb windows, we can leave the loop when finding the
> +      // first non-popup window. If however the current window is in private
> +      // browsing mode then we need at least one other pb and one non-popup

unrelated change

Comment 4

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch ver.2

Rebase & fixed.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 647426 [details] [diff] [review]
patch ver.2

Thank you!

Comment 6

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf51689ab43e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cf51689ab43e