Remove the windowdragbox element and binding

RESOLVED FIXED in Firefox 60

Status

()

enhancement
RESOLVED FIXED
a year ago
5 months ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks 1 bug)

Trunk
mozilla60
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
The element is used only in pageInfo.xul and the binding doesn't do anything meaningful on Windows and Mac: https://searchfox.org/mozilla-central/rev/6cf5ac594a21db6df359dc49f4aa651fef59a27b/toolkit/modules/WindowDraggingUtils.jsm#7,14-16
(Assignee)

Updated

a year ago
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8955852 [details]
Bug 1442961 - Remove the windowdragbox element and binding.

https://reviewboard.mozilla.org/r/224870/#review230914


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/base/content/browser.js:1548
(Diff revision 1)
>  
>      gNavToolbox.addEventListener("customizationstarting", CustomizationHandler);
>      gNavToolbox.addEventListener("customizationending", CustomizationHandler);
>  
> +    if (AppConstants.platform == "linux") {
> +      let tmp = {};

Error: 'tmp' is already declared in the upper scope. [eslint: no-shadow]
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8955852 [details]
Bug 1442961 - Remove the windowdragbox element and binding.

https://reviewboard.mozilla.org/r/224870/#review230958

::: browser/base/content/browser.js:1546
(Diff revision 2)
> +    if (AppConstants.platform == "linux") {
> +      let { WindowDraggingElement } =
> +        ChromeUtils.import("resource://gre/modules/WindowDraggingUtils.jsm", {});
> +      new WindowDraggingElement(document.getElementById("titlebar"));
> +    }

Shouldn't this be done in pageInfo.js as well ?
(Assignee)

Comment 5

a year ago
(In reply to Tim Nguyen :ntim from comment #4)
> Comment on attachment 8955852 [details]
> Bug 1442961 - Remove the windowdragbox element and binding.
> 
> https://reviewboard.mozilla.org/r/224870/#review230958
> 
> ::: browser/base/content/browser.js:1546
> (Diff revision 2)
> > +    if (AppConstants.platform == "linux") {
> > +      let { WindowDraggingElement } =
> > +        ChromeUtils.import("resource://gre/modules/WindowDraggingUtils.jsm", {});
> > +      new WindowDraggingElement(document.getElementById("titlebar"));
> > +    }
> 
> Shouldn't this be done in pageInfo.js as well ?

No. The windowdragbox doesn't do anything on Linux.

Comment 6

a year ago
mozreview-review
Comment on attachment 8955852 [details]
Bug 1442961 - Remove the windowdragbox element and binding.

https://reviewboard.mozilla.org/r/224870/#review231058
Attachment #8955852 - Flags: review?(jaws) → review+

Comment 7

a year ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c78fcb403361
Remove the windowdragbox element and binding. r=jaws
https://hg.mozilla.org/mozilla-central/rev/c78fcb403361
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(Assignee)

Updated

6 months ago
See Also: → 1500424
See Also: → 1507863
You need to log in before you can comment on or make changes to this bug.