Use handleEvent() for BrowserOnClick

RESOLVED FIXED in Firefox 17

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tetsuharu, Assigned: tetsuharu)

Tracking

Trunk
Firefox 17
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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

Updated

5 years ago
Assignee: nobody → saneyuki.s.snyk
(Assignee)

Updated

5 years ago
Component: Developer Tools → General
Created attachment 646826 [details] [diff] [review]
patch
Attachment #646826 - Flags: review?(josh)

Comment 2

5 years ago
Comment on attachment 646826 [details] [diff] [review]
patch

I am not a browser peer, so I'm going to randomly select Felipe.
Attachment #646826 - Flags: review?(josh) → review?(felipc)
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
Attachment #646826 - Flags: review?(felipc) → review+
Created attachment 647426 [details] [diff] [review]
patch ver.2

Rebase & fixed.
Attachment #647426 - Flags: review?(felipc)
Comment on attachment 647426 [details] [diff] [review]
patch ver.2

Thank you!
Attachment #647426 - Flags: review?(felipc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf51689ab43e
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 17
https://hg.mozilla.org/mozilla-central/rev/cf51689ab43e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.