PanelMultiView.jsm: replace _dwu getter with _getBoundsWithoutFlushing method

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: dao, Assigned: sahilbhosale63, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

(Whiteboard: [lang=js])

Attachments

(1 attachment, 12 obsolete attachments)

(Reporter)

Description

10 months ago
+++ This bug was initially created as a clone of Bug #1480982 +++

In PanelMultiView.jsm, every place where _dwu is used ends up calling getBoundsWithoutFlushing:

  foo = this._dwu.getBoundsWithoutFlushing(...);

So let's replace this:

https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/browser/components/customizableui/PanelMultiView.jsm#175-182

with a new _getBoundsWithoutFlushing method:

  _getBoundsWithoutFlushing(element) {
    return this.window.windowUtils.getBoundsWithoutFlushing(element);
  },

and replace the _dwu users with a call to our new method:

  foo = this._getBoundsWithoutFlushing(...);
(Assignee)

Comment 1

10 months ago
I want to work on this issue.
(Reporter)

Comment 2

10 months ago
(In reply to sahilbhosale63 from comment #1)
> I want to work on this issue.

Go for it! Do you have the Firefox source code already? Let me know if you have questions.
(Assignee)

Comment 3

10 months ago
Yes, I already have the source code running on my machine.
(Assignee)

Comment 4

10 months ago
Posted patch tip.patch (obsolete) — Splinter Review
Bug 1481401 - replace _dwr getter with _getBoundsWithoutFlushing method. r?dao
(Reporter)

Comment 5

10 months ago
Comment on attachment 8998425 [details] [diff] [review]
tip.patch

Next time when attaching a patch that you want to get reviewed, please make sure to set the review flag.

>@@ -1368,7 +1368,7 @@ var PanelView = class extends Associated
>       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
>     let dwu = this._dwu;
>     return buttons.filter(button => {
>-      let bounds = dwu.getBoundsWithoutFlushing(button);
>+      let bounds = _getBoundsWithoutFlushing(button);
>       return bounds.width > 0 && bounds.height > 0;
>     });

You need to remove the dwu variable and call this._getBoundsWithoutFlushing rather than _getBoundsWithoutFlushing in the filter function.

You also still need to replace the _dwu implementation (https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/browser/components/customizableui/PanelMultiView.jsm#175-182) with the _getBoundsWithoutFlushing implementation I posted in comment 0.
(Reporter)

Updated

10 months ago
Assignee: nobody → sahilbhosale63
(Assignee)

Comment 6

10 months ago
Posted patch tip.patch (obsolete) — Splinter Review
Bug 1481401- replaced -dwu getter with _getBoundsWithoutFlushing method. r?dao
Attachment #8998453 - Flags: review+
(Reporter)

Comment 7

10 months ago
Comment on attachment 8998453 [details] [diff] [review]
tip.patch

>   /**
>    * nsIDOMWindowUtils for the window of this node.
>    */

This comment is now obsolete, please remove it.

>-  get _dwu() {
>-    if (this.__dwu)
>-      return this.__dwu;
>-    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>+  get _getBoundsWithoutFlushing() {
>+    if (this.__getBoundsWithoutFlushing)
>+      return this.__getBoundsWithoutFlushing;
>+    return this.__getBoundsWithoutFlushing = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>                                    .getInterface(Ci.nsIDOMWindowUtils);
>   }

No need for __getBoundsWithoutFlushing, just return this.window.windowUtils directly.

> 
>@@ -424,7 +424,7 @@ var PanelMultiView = class extends Assoc
>     this._panel.removeEventListener("popuphidden", this);
>     this.window.removeEventListener("keydown", this);
>     this.node = this._openPopupPromise = this._openPopupCancelCallback =
>-      this._viewContainer = this._viewStack = this.__dwu =
>+      this._viewContainer = this._viewStack = this.__getBoundsWithoutFlushing =
>       this._transitionDetails = null;
>   }

Please remove __getBoundsWithoutFlushing here as well.

>@@ -1366,9 +1366,9 @@ var PanelView = class extends Associated
>   getNavigableElements() {
>     let buttons = Array.from(this.node.querySelectorAll(
>       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
>-    let dwu = this._dwu;
>+    

nit: remove spaces from the empty line
Attachment #8998453 - Flags: review+
(Reporter)

Comment 9

10 months ago
Comment on attachment 8998460 [details] [diff] [review]
removed the comments and blank spaces, and changed the things which you said.

>--- a/browser/components/customizableui/PanelMultiView.jsm
>+++ b/browser/components/customizableui/PanelMultiView.jsm
>@@ -175,10 +175,10 @@ var AssociatedToNode = class {
>-  get _dwu() {
>-    if (this.__dwu)
>-      return this.__dwu;
>-    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>+  get _getBoundsWithoutFlushing() {
>+    if (this.__getBoundsWithoutFlushing)
>+      return this.__getBoundsWithoutFlushing;
>+    return this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>                                    .getInterface(Ci.nsIDOMWindowUtils);
>   }

The _dwu getter doesn't contain the QueryInterface anymore. Is your mozilla-central copy outdated?

>@@ -424,7 +424,7 @@ var PanelMultiView = class extends Assoc
>     this._panel.removeEventListener("popuphidden", this);
>     this.window.removeEventListener("keydown", this);
>     this.node = this._openPopupPromise = this._openPopupCancelCallback =
>-      this._viewContainer = this._viewStack = this.__dwu =
>+      this._viewContainer = this._viewStack = this._transitionDetails = null;
>   }

This looks broken, _transitionDetails seems to be coming out of nowhere. Did you manually edit the patch?

Btw, please set the review flag to ? rather than +.
Attachment #8998460 - Flags: review+
(Assignee)

Comment 10

10 months ago
Posted patch tip.patch (obsolete) — Splinter Review
I by mistake edited the tip.patch file directly, sorry for that. Now, I have  . changed the source file and have done what you said to do.
Attachment #8998464 - Flags: review?(dao+bmo)
(Reporter)

Comment 11

10 months ago
Comment on attachment 8998464 [details] [diff] [review]
tip.patch

>diff --git a/browser/components/customizableui/PanelMultiView.jsm b/browser/components/customizableui/PanelMultiView.jsm
>--- a/browser/components/customizableui/PanelMultiView.jsm
>+++ b/browser/components/customizableui/PanelMultiView.jsm
>@@ -172,13 +172,10 @@ var AssociatedToNode = class {
>     return this.node.ownerGlobal;
>   }
> 
>-  /**
>-   * nsIDOMWindowUtils for the window of this node.
>-   */
>-  get _dwu() {
>-    if (this.__dwu)
>-      return this.__dwu;
>-    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>+  get _getBoundsWithoutFlushing() {
>+    if (this.__getBoundsWithoutFlushing)
>+      return this.__getBoundsWithoutFlushing;
>+    return this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>                                    .getInterface(Ci.nsIDOMWindowUtils);
>   }

See my previous comment -- your mozilla-central copy looks outdated. You can update it with hg pull -u.

>   getNavigableElements() {
>     let buttons = Array.from(this.node.querySelectorAll(
>       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
>-    let dwu = this._dwu;
>+    

Please remove the spaces from the empty line.
Attachment #8998464 - Flags: review?(dao+bmo)
(Assignee)

Comment 12

10 months ago
Posted patch tip.patch (obsolete) — Splinter Review
Updated my source code and removed the extra space.
Attachment #8998468 - Flags: review?(dao+bmo)
(Reporter)

Updated

10 months ago
Attachment #8998425 - Attachment is obsolete: true
(Reporter)

Updated

10 months ago
Attachment #8998464 - Attachment is obsolete: true
(Reporter)

Updated

10 months ago
Attachment #8998453 - Attachment is obsolete: true
(Reporter)

Updated

10 months ago
Attachment #8998460 - Attachment is obsolete: true
(Reporter)

Comment 13

10 months ago
Comment on attachment 8998468 [details] [diff] [review]
tip.patch

>-  /**
>-   * nsIDOMWindowUtils for the window of this node.
>-   */
>-  get _dwu() {
>-    if (this.__dwu)
>-      return this.__dwu;
>-    return this.__dwu = this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>+  get _getBoundsWithoutFlushing() {
>+    if (this.__getBoundsWithoutFlushing)
>+      return this.__getBoundsWithoutFlushing;
>+    return this.window.QueryInterface(Ci.nsIInterfaceRequestor)
>                                    .getInterface(Ci.nsIDOMWindowUtils);
>   }

This is still not up to date. The _dwu getter should have looked like this:

  /**
   * nsIDOMWindowUtils for the window of this node.
   */
  get _dwu() {
    if (this.__dwu)
      return this.__dwu;
    return this.__dwu = this.window.windowUtils;
  }
Attachment #8998468 - Flags: review?(dao+bmo)
(Assignee)

Comment 14

10 months ago
Posted patch final.patch (obsolete) — Splinter Review
I have changed _dwu with _getBoundsWithoutFlushing because i thought i also have to change it.
Attachment #8998471 - Flags: review?(dao+bmo)
(Reporter)

Comment 15

10 months ago
(In reply to Sahil Bhosale from comment #14)
> Created attachment 8998471 [details] [diff] [review]
> final.patch
> 
> I have changed _dwu with _getBoundsWithoutFlushing because i thought i also
> have to change it.

Again, the goal is to replace this:

>   /**
>    * nsIDOMWindowUtils for the window of this node.
>    */
>   get _dwu() {
>     if (this.__dwu)
>       return this.__dwu;
>     return this.__dwu = this.window.windowUtils;
>   }

With this:

>  _getBoundsWithoutFlushing(element) {
>    return this.window.windowUtils.getBoundsWithoutFlushing(element);
>  },
(Reporter)

Updated

10 months ago
Attachment #8998468 - Attachment is obsolete: true
(Reporter)

Updated

10 months ago
Attachment #8998471 - Flags: review?(dao+bmo)
(Assignee)

Comment 16

10 months ago
Attachment #8998534 - Flags: review?(dao+bmo)
(Reporter)

Comment 17

9 months ago
Comment on attachment 8998534 [details] [diff] [review]
replaced the dwu method with _getBoundsWithoutFlushing

>@@ -1366,13 +1359,11 @@ var PanelView = class extends Associated
>   getNavigableElements() {
>     let buttons = Array.from(this.node.querySelectorAll(
>       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
>-    let dwu = this._dwu;
>     return buttons.filter(button => {
>-      let bounds = _getBoundsWithoutFlushing(button);

I don't see where this line is coming from... Did you manually edit the patch file?

>       return bounds.width > 0 && bounds.height > 0;
>     });
>   }
>-
>   /**

Please leave the empty line.
Attachment #8998534 - Flags: review?(dao+bmo)
(Assignee)

Comment 18

9 months ago
No, I have not edited it manually.
(Reporter)

Comment 19

9 months ago
Maybe you committed some other changes then, before creating the patch.

Your patch says you removed this line:

>-      let bounds = _getBoundsWithoutFlushing(button);

But PanelMultiView.jsm doesn't have that line in the first place. What it does have is this line:

>      let bounds = dwu.getBoundsWithoutFlushing(element);

https://searchfox.org/mozilla-central/rev/0f4d50ff5211e8dd85e119ef683d04b63062ed90/browser/components/customizableui/PanelMultiView.jsm#1382
(Assignee)

Comment 20

9 months ago
Posted patch final.patch (obsolete) — Splinter Review
I solved that extra space problem. The reason why that space was there is because i haven't left an empty line between the code and the comment. I think now it is done.
Attachment #8998775 - Flags: review?(dao+bmo)
(Reporter)

Comment 21

9 months ago
Comment on attachment 8998775 [details] [diff] [review]
final.patch

Okay, this is starting to look really good, except... is your mozilla-central copy still outdated?

For instance, you patched this method:

>   getNavigableElements() {
>     let buttons = Array.from(this.node.querySelectorAll(
>       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
>-    let dwu = this._dwu;
>     return buttons.filter(button => {
>-      let bounds = _getBoundsWithoutFlushing(button);
>+      let bounds = this._getBoundsWithoutFlushing(button);
>       return bounds.width > 0 && bounds.height > 0;
>     });
>   }

But getNavigableElements doesn't exist anymore. There's a _navigableElements getter instead:

https://searchfox.org/mozilla-central/rev/0f4d50ff5211e8dd85e119ef683d04b63062ed90/browser/components/customizableui/PanelMultiView.jsm#1366
Attachment #8998775 - Flags: review?(dao+bmo)
(Reporter)

Updated

9 months ago
Attachment #8998471 - Attachment is obsolete: true
(Reporter)

Updated

9 months ago
Attachment #8998534 - Attachment is obsolete: true
(Assignee)

Comment 22

9 months ago
(In reply to Dão Gottwald [::dao] from comment #21)
> Comment on attachment 8998775 [details] [diff] [review]
> final.patch
> 
> Okay, this is starting to look really good, except... is your
> mozilla-central copy still outdated?
> 
> For instance, you patched this method:
> 
> >   getNavigableElements() {
> >     let buttons = Array.from(this.node.querySelectorAll(
> >       ".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
> >-    let dwu = this._dwu;
> >     return buttons.filter(button => {
> >-      let bounds = _getBoundsWithoutFlushing(button);
> >+      let bounds = this._getBoundsWithoutFlushing(button);
> >       return bounds.width > 0 && bounds.height > 0;
> >     });
> >   }
> 
> But getNavigableElements doesn't exist anymore. There's a _navigableElements
> getter instead:
> 
> https://searchfox.org/mozilla-central/rev/
> 0f4d50ff5211e8dd85e119ef683d04b63062ed90/browser/components/customizableui/
> PanelMultiView.jsm#1366

I have updated my mozilla-centeral now using hg pull -u command but still it shows me the getNavigationElements() method instead of _navigationElements. What should I do?
(Reporter)

Comment 23

9 months ago
Did hg pull -u succeed? What was its output?
(Assignee)

Comment 24

9 months ago
This was the output of hg pull -u command:

pulling from https://hg.mozilla.org/mozilla-central
searching for changes
adding changesets
adding manifests                                                                           
adding file changes
added 323 changesets with 2001 changes to 1379 files                                       
new changesets ec96693b39be:f650c0df72f9
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
updated to "3dd40d95c802: Bug 1481401 - replace _dwu getter with _getBoundsWithoutFlushing method. r?dao"
1 other heads for branch "default"


Here it is saying that 0 files updated and I don't know why it is mentioning the bug no.
(Reporter)

Comment 25

9 months ago
You created another head because you had a local commit before updating. Maybe try:

hg strip 3dd40d95c802 && hg pull && hg up
(Assignee)

Comment 26

9 months ago
(In reply to Dão Gottwald [::dao] from comment #25)
> You created another head because you had a local commit before updating.
> Maybe try:
> 
> hg strip 3dd40d95c802 && hg pull && hg up

It says unknown command 'strip' see this:

hg: unknown command 'strip'
'strip' is provided by the following extension:

    strip         strip changesets and their descendants from history

(use 'hg help extensions' for information on enabling extensions)
(Reporter)

Comment 27

9 months ago
What does hg heads output?
(Assignee)

Comment 28

9 months ago
(In reply to Dão Gottwald [::dao] from comment #27)
> What does hg heads output?

changeset:   430751:f650c0df72f9
tag:         tip
parent:      430595:eb9ff7de69ef
parent:      430750:21cbd16b3b27
user:        Tiberius Oros <toros@mozilla.com>
date:        Thu Aug 09 13:02:05 2018 +0300
summary:     Merge inbound to mozilla-central.  a=merge

changeset:   422984:3dd40d95c802
user:        Sahil <sahilbhosale63@live.com>
date:        Wed Aug 08 00:21:35 2018 +0530
summary:     Bug 1481401 - replace _dwu getter with _getBoundsWithoutFlushing method. r?dao
(Reporter)

Comment 29

9 months ago
Ah, you didn't have the strip extension enabled. Open your ~/.hgrc file and add a line saying "strip = " (without quotes) under [extensions], then try the command from comment 25 again.
(Assignee)

Comment 30

9 months ago
I my system when i run:

1. ~/.hgrc : It gives output permission denied.

2. hg config -e : It runs successfully and I typed the (strip = ) in the [extensions].

3. hg config -e -g : It gives the output no such file or directory.  


After running the 2. command shown above, I ran the command in comment 25 i.e: hg strip 3dd40d95c802 && hg pull && hg up which gives following output:

abort: local changes found
(Reporter)

Comment 31

9 months ago
(In reply to Sahil Bhosale from comment #30)
> I my system when i run:
> 
> 1. ~/.hgrc : It gives output permission denied.

It's a text file, you'd need to open it with a text editor rather than executing it.

> 2. hg config -e : It runs successfully and I typed the (strip = ) in the
> [extensions].
> 
> 3. hg config -e -g : It gives the output no such file or directory.  
> 
> 
> After running the 2. command shown above, I ran the command in comment 25
> i.e: hg strip 3dd40d95c802 && hg pull && hg up which gives following output:
> 
> abort: local changes found

Try:

hg up -C && strip 3dd40d95c802 && hg pull && hg up

Note that this will remove your local changes.
(Assignee)

Comment 32

9 months ago
> Try:
> 
> hg up -C && strip 3dd40d95c802 && hg pull && hg up
> 
> Note that this will remove your local changes.

This was the output for the above command:

0 files updated, 0 files merged, 0 files removed, 0 files unresolved
updated to "3dd40d95c802: Bug 1481401 - replace _dwu getter with _getBoundsWithoutFlushing method. r?dao"
1 other heads for branch "default"
strip: '3dd40d95c802': No such file
(Assignee)

Comment 33

9 months ago
What is the progress of this bug does it has been solved or some things are still remaining?
there's "hg" missing before "strip"

try this:

hg strip 3dd40d95c802 && hg pull && hg up
(Assignee)

Comment 35

9 months ago
Posted patch final.patch (obsolete) — Splinter Review
Updated the source code and replaced the dwu method.
Attachment #8998849 - Flags: review?(dao+bmo)
(Assignee)

Comment 36

9 months ago
Posted patch final.patch (obsolete) — Splinter Review
replaced all the dwu method with getBoundsWithoutFlushing().
Attachment #8998860 - Flags: review?(dao+bmo)
(Reporter)

Updated

9 months ago
Attachment #8998849 - Attachment is obsolete: true
Attachment #8998849 - Flags: review?(dao+bmo)
(Reporter)

Updated

9 months ago
Attachment #8998775 - Attachment is obsolete: true
(Reporter)

Comment 37

9 months ago
Comment on attachment 8998860 [details] [diff] [review]
final.patch

>@@ -423,8 +418,7 @@ var PanelMultiView = class extends Assoc
>     this._panel.removeEventListener("popuphidden", this);
>     this.window.removeEventListener("keydown", this);
>     this.node = this._openPopupPromise = this._openPopupCancelCallback =
>-      this._viewContainer = this._viewStack = this.__dwu =
>-      this._transitionDetails = null;
>+      this._viewContainer = this._viewStack = this._transitionDetails = null;
>   }
> 
>   /**
>@@ -878,7 +872,7 @@ var PanelMultiView = class extends Assoc
>       let header = viewNode.firstChild;
>       if (header && header.classList.contains("panel-header")) {
>         viewRect.height += await window.promiseDocumentFlushed(() => {
>-          return this._dwu.getBoundsWithoutFlushing(header).height;
>+          return this.getBoundsWithoutFlushing(header).height;
>         });
>       }
>       await nextPanelView.descriptionHeightWorkaround();
>@@ -892,7 +886,7 @@ var PanelMultiView = class extends Assoc
>       await nextPanelView.descriptionHeightWorkaround();
> 
>       viewRect = await window.promiseDocumentFlushed(() => {
>-        return this._dwu.getBoundsWithoutFlushing(viewNode);
>+        return this.getBoundsWithoutFlushing(viewNode);
>       });
>       // Bail out if the panel was closed in the meantime.
>       if (!nextPanelView.isOpenIn(this)) {
>@@ -1257,7 +1251,7 @@ var PanelView = class extends Associated
>    * navigation if the view is still open but is invisible.
>    */
>   captureKnownSize() {
>-    let rect = this._dwu.getBoundsWithoutFlushing(this.node);
>+    let rect = this.getBoundsWithoutFlushing(this.node);
>     this.knownWidth = rect.width;
>     this.knownHeight = rect.height;
>   }
>@@ -1370,7 +1364,6 @@ var PanelView = class extends Associated
> 
>     let navigableElements = Array.from(this.node.querySelectorAll(
>       ":-moz-any(button,toolbarbutton,menulist,.text-link):not([disabled])"));
>-    let dwu = this._dwu;
>     return this.__navigableElements = navigableElements.filter(element => {
>       // Set the "tabindex" attribute to make sure the element is focusable.
>       if (!element.hasAttribute("tabindex")) {
>@@ -1379,7 +1372,7 @@ var PanelView = class extends Associated
>       if (element.hasAttribute("disabled")) {
>         return false;
>       }
>-      let bounds = dwu.getBoundsWithoutFlushing(element);
>+      let bounds = this.getBoundsWithoutFlushing(element);
>       return bounds.width > 0 && bounds.height > 0;
>     });
>   }

These should all call this._getBoundsWithoutFlushing rather than this.getBoundsWithoutFlushing.
Attachment #8998860 - Flags: review?(dao+bmo)
(Assignee)

Comment 38

9 months ago
Posted patch final.patch (obsolete) — Splinter Review
Changes done.
Attachment #8998898 - Flags: review?(dao+bmo)
(Reporter)

Comment 39

9 months ago
Comment on attachment 8998898 [details] [diff] [review]
final.patch

>     return this.node.ownerGlobal;
>   }
> 
>-  /**
>-   * nsIDOMWindowUtils for the window of this node.
>-   */
>-  get _dwu() {
>-    if (this.__dwu)
>-      return this.__dwu;
>-    return this.__dwu = this.window.windowUtils;
>-  }
>+  _getBoundsWithoutFlushing(element) {
>+    return this.window.windowUtils._getBoundsWithoutFlushing(element);

Here you need to call this.window.windowUtils.getBoundsWithoutFlushing, not this.window.windowUtils._getBoundsWithoutFlushing.
Attachment #8998898 - Flags: review?(dao+bmo)
(Reporter)

Updated

9 months ago
Attachment #8998860 - Attachment is obsolete: true
(Assignee)

Comment 40

9 months ago
Posted patch final.patch (obsolete) — Splinter Review
Done, now the toolbar is working properly.
Attachment #8998956 - Flags: review?(dao+bmo)
(Reporter)

Comment 41

9 months ago
Comment on attachment 8998956 [details] [diff] [review]
final.patch

Looks good, thanks!
Attachment #8998956 - Flags: review?(dao+bmo) → review+
(Reporter)

Updated

9 months ago
Attachment #8998898 - Attachment is obsolete: true
(Reporter)

Updated

9 months ago
Keywords: checkin-needed
(Assignee)

Comment 42

9 months ago
Can you just guide me here, what I have to do next or is my job done?
Flags: needinfo?(dao+bmo)
Hi Sahil,

dao has reviewed your patch and given it an r+, and has also set the checkin-needed flag. That means that, within a few hours, someone will come along and land your patch for you and eventually (assuming the tests pass in our continuous integration branch), this bug will be marked RESOLVED FIXED.

So at this point, your job is done! Congrats! :)
Flags: needinfo?(dao+bmo)

Comment 44

9 months ago
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/435661644b2d
"PanelMultiView.jsm: replace _dwu getter with _getBoundsWithoutFlushing method" [r=dao+bmo]
Keywords: checkin-needed
(Reporter)

Comment 46

9 months ago
Sahil, your patch still had trailing spaces and so it didn't land successfully. Can you please remove these spaces?

+  _getBoundsWithoutFlushing(element) {
+    return this.window.windowUtils.getBoundsWithoutFlushing(element);
+  }   <--
(Assignee)

Comment 47

9 months ago
(In reply to Dão Gottwald [::dao] from comment #46)
> Sahil, your patch still had trailing spaces and so it didn't land
> successfully. Can you please remove these spaces?
> 
> +  _getBoundsWithoutFlushing(element) {
> +    return this.window.windowUtils.getBoundsWithoutFlushing(element);
> +  }   <--

Yes, I will soon remove the trailing spaces, I was in college so haven't done that. Now I will do that.
Flags: needinfo?(sahilbhosale63)
(Assignee)

Comment 48

9 months ago
Posted patch final.patchSplinter Review
Removed the trailing space.
Attachment #8999138 - Flags: review?(dao+bmo)
(Reporter)

Updated

9 months ago
Attachment #8999138 - Flags: review?(dao+bmo) → review+
(Reporter)

Updated

9 months ago
Attachment #8998956 - Attachment is obsolete: true
(Reporter)

Updated

9 months ago
Keywords: checkin-needed

Comment 49

9 months ago
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d0483ecd593
PanelMultiView.jsm: replace _dwu getter with _getBoundsWithoutFlushing method. r=dao
Keywords: checkin-needed

Comment 50

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4d0483ecd593
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.