1286854 - Replace ownerDocument.defaultView with ownerGlobal in browser/

Status: RESOLVED FIXED

(Firefox :: General, defect, P3)

Description

[Dão Gottwald [:dao]]

We should use the new and simpler ownerGlobal property instead of ownerDocument.defaultView.

Affected files:

https://dxr.mozilla.org/mozilla-central/search?q=ownerdocument.defaultview+path%3Abrowser%2F&redirect=false

Comment 1

[pushpankark]

Please assign it to me.

Comment 2

[pushpankark]

Attachment: renamed ownerDocument.defaultView to ownerGlobal

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8771386 [details] [diff] [review]
renamed ownerDocument.defaultView to ownerGlobal

This patch contains a bunch of changes outside of browser/ that are beyond this bug's scope. Can you please make a patch for browser/ only?

Comment 4

[pushpankark]

Ok

Comment 5

[pushpankark]

Attachment: renamed ownerDocument.defaultView to ownerGlobal

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8771393 [details] [diff] [review]
renamed ownerDocument.defaultView to ownerGlobal

>--- a/browser/base/content/content.js
>+++ b/browser/base/content/content.js

>   getMediaItems: function(document, strings, elem)
>   {
>     // Check for images defined in CSS (e.g. background, borders)
>-    let computedStyle = elem.ownerDocument.defaultView.getComputedStyle(elem, "");
>+    let computedStyle = elem.ownerGlobal.getComputedStyle(elem, "");

please remove getComputedStyle's second argument (the empty string) while you're touching this line

>--- a/browser/base/content/test/general/browser_bug431826.js
>+++ b/browser/base/content/test/general/browser_bug431826.js

>   yield remote(() => {
>     let div = content.document.getElementById("badCertAdvancedPanel");
>     // Confirm that the expert section is collapsed
>     Assert.ok(div, "Advanced content div should exist");
>-    Assert.equal(div.ownerDocument.defaultView.getComputedStyle(div, "").display,
>+    Assert.equal(div.ownerGlobal.getComputedStyle(div, "").display,

same here

>   yield remote(() => {
>     let div = content.document.getElementById("badCertAdvancedPanel");
>     Assert.ok(div, "Advanced content div should exist");
>-    Assert.equal(div.ownerDocument.defaultView.getComputedStyle(div, "").display,
>+    Assert.equal(div.ownerGlobal.getComputedStyle(div, "").display,

and here too

>--- a/browser/base/content/test/general/head.js
>+++ b/browser/base/content/test/general/head.js

> function is_hidden(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

ditto

> function is_visible(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

ditto

>--- a/browser/base/content/test/urlbar/head.js
>+++ b/browser/base/content/test/urlbar/head.js

> function is_hidden(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

ditto

> function is_visible(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

ditto

>--- a/browser/components/customizableui/CustomizableUI.jsm
>+++ b/browser/components/customizableui/CustomizableUI.jsm

>   getCustomizeTargetForArea: function(aArea, aWindow) {
>     let buildAreaNodes = gBuildAreas.get(aArea);
>     if (!buildAreaNodes) {
>       return null;
>     }
> 
>     for (let node of buildAreaNodes) {
>-      if (node.ownerDocument.defaultView === aWindow) {
>+      if (node.ownerGlobal === aWindow) {

please replace === with == while you're touching this line

>--- a/browser/components/customizableui/CustomizableWidgets.jsm
>+++ b/browser/components/customizableui/CustomizableWidgets.jsm

>     onCommand: function(e) {
>-      if (e.target && e.target.ownerDocument && e.target.ownerDocument.defaultView) {
>-        let win = e.target.ownerDocument.defaultView;
>+      if (e.target && e.target.ownerDocument && e.target.ownerGlobal) {
>+        let win = e.target.ownerGlobal;
>         if (typeof win.OpenBrowserWindow == "function") {
>           win.OpenBrowserWindow({private: true});
>         }
>       }
>     }

this could be simplified to:

    onCommand: function(e) {
      let win = e.target && e.target.ownerGlobal;
      if (win && typeof win.OpenBrowserWindow == "function") {
        win.OpenBrowserWindow({private: true});
      }
    }

>     id: "save-page-button",
>     shortcutId: "key_savePage",
>     tooltiptext: "save-page-button.tooltiptext3",
>     defaultArea: CustomizableUI.AREA_PANEL,
>     onCommand: function(aEvent) {
>       let win = aEvent.target &&
>                 aEvent.target.ownerDocument &&
>-                aEvent.target.ownerDocument.defaultView;
>+                aEvent.target.ownerGlobal;

"aEvent.target.ownerDocument &&" is redundant now

>     id: "find-button",
>     shortcutId: "key_find",
>     tooltiptext: "find-button.tooltiptext3",
>     defaultArea: CustomizableUI.AREA_PANEL,
>     onCommand: function(aEvent) {
>       let win = aEvent.target &&
>                 aEvent.target.ownerDocument &&
>-                aEvent.target.ownerDocument.defaultView;
>+                aEvent.target.ownerGlobal;

ditto

>   }, {
>     id: "open-file-button",
>     shortcutId: "openFileKb",
>     tooltiptext: "open-file-button.tooltiptext3",
>     defaultArea: CustomizableUI.AREA_PANEL,
>     onCommand: function(aEvent) {
>       let win = aEvent.target
>                 && aEvent.target.ownerDocument
>-                && aEvent.target.ownerDocument.defaultView;
>+                && aEvent.target.ownerGlobal;

ditto

>     id: "add-ons-button",
>     shortcutId: "key_openAddons",
>     tooltiptext: "add-ons-button.tooltiptext3",
>     defaultArea: CustomizableUI.AREA_PANEL,
>     onCommand: function(aEvent) {
>       let win = aEvent.target &&
>                 aEvent.target.ownerDocument &&
>-                aEvent.target.ownerDocument.defaultView;
>+                aEvent.target.ownerGlobal;

ditto

> let preferencesButton = {
>   id: "preferences-button",
>   defaultArea: CustomizableUI.AREA_PANEL,
>   onCommand: function(aEvent) {
>     let win = aEvent.target &&
>               aEvent.target.ownerDocument &&
>-              aEvent.target.ownerDocument.defaultView;
>+              aEvent.target.ownerGlobal;

ditto

>--- a/browser/components/extensions/ext-tabs.js
>+++ b/browser/components/extensions/ext-tabs.js

>           let numPinned = gBrowser._numPinnedTabs;
>           let ok = tab.pinned ? insertionPoint <= numPinned : insertionPoint >= numPinned;
>           if (!ok) {
>             continue;
>           }
> 
>           indexMap.set(window, insertionPoint + 1);
> 
>-          if (tab.ownerDocument.defaultView !== window) {
>+          if (tab.ownerGlobal !== window) {

please replace !== with != while you're touching this line

>--- a/browser/components/preferences/in-content/tests/head.js
>+++ b/browser/components/preferences/in-content/tests/head.js

> function is_hidden(aElement) {
>-  var style = aElement.ownerDocument.defaultView.getComputedStyle(aElement, "");
>+  var style = aElement.ownerGlobal.getComputedStyle(aElement, "");

again, please remove getComputedStyle's second argument since it's not required

>--- a/browser/components/sessionstore/SessionStore.jsm
>+++ b/browser/components/sessionstore/SessionStore.jsm

>     // Flush to get the latest tab state to duplicate.
>     let browser = aTab.linkedBrowser;
>     TabStateFlusher.flush(browser).then(() => {
>       // The new tab might have been closed in the meantime.
>       if (newTab.closing || !newTab.linkedBrowser) {
>         return;
>       }
> 
>-      let window = newTab.ownerDocument && newTab.ownerDocument.defaultView;
>+      let window = newTab.ownerDocument && newTab.ownerGlobal;

again, "newTab.ownerDocument &&" can be removed now

>         this._remotenessChangingBrowsers.get(browser.permanentKey);
>       this._remotenessChangingBrowsers.delete(browser.permanentKey);
> 
>       // The tab might have been closed/gone in the meantime.
>       if (tab.closing || !tab.linkedBrowser) {
>         return;
>       }
> 
>-      let window = tab.ownerDocument && tab.ownerDocument.defaultView;
>+      let window = tab.ownerDocument && tab.ownerGlobal;

ditto

>--- a/browser/components/sessionstore/content/content-sessionStore.js
>+++ b/browser/components/sessionstore/content/content-sessionStore.js
>@@ -414,17 +414,17 @@ var FormDataListener = {
>     addEventListener("input", this, true);
>     addEventListener("change", this, true);
>     gFrameTree.addObserver(this);
>   },
> 
>   handleEvent: function (event) {
>     let frame = event.target &&
>                 event.target.ownerDocument &&
>-                event.target.ownerDocument.defaultView;
>+                event.target.ownerGlobal;

ditto

>--- a/browser/components/uitour/test/head.js
>+++ b/browser/components/uitour/test/head.js
>@@ -49,33 +49,33 @@ function taskify(fun) {
>     return Task.spawn(fun).then(done, (reason) => {
>       ok(false, reason);
>       done();
>     });
>   };
> }
> 
> function is_hidden(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

as before, please remove getComputedStyle's redundant second argument while you're touching this line

> function is_visible(element) {
>-  var style = element.ownerDocument.defaultView.getComputedStyle(element, "");
>+  var style = element.ownerGlobal.getComputedStyle(element, "");

ditto

>--- a/browser/modules/ContentSearch.jsm
>+++ b/browser/modules/ContentSearch.jsm
>@@ -227,17 +227,17 @@ this.ContentSearch = {
>       "healthReportKey",
>       "searchPurpose",
>     ]);
>     let engine = Services.search.getEngineByName(data.engineName);
>     let submission = engine.getSubmission(data.searchString, "", data.searchPurpose);
>     let browser = msg.target;
>     let win;
>     try {
>-      win = browser.ownerDocument.defaultView;
>+      win = browser.ownerGlobal;
>     }
>     catch (err) {
>       // The browser may have been closed between the time its content sent the
>       // message and the time we handle it.  In that case, trying to call any
>       // method on it will throw.
>       return;
>     }

I don't think this exception will throw anymore with your change, so please rewrite this block like this:

    let win = browser.ownerGlobal;
    if (!win) {
      // The browser may have been closed between the time its content sent the
      // message and the time we handle it.
      return;
    }

>--- a/browser/extensions/pocket/content/panels/js/vendor/jquery-2.1.1.min.js
>+++ b/browser/extensions/pocket/content/panels/js/vendor/jquery-2.1.1.min.js

please don't modify this file

Looks good otherwise.

Comment 7

[pushpankark]

Can i give these new modification in a new patch

Comment 8

[Dão Gottwald [:dao]]

Yes, you could do that.

Comment 9

[pushpankark]

Attachment: Bug fix modifications.

Comment 10

[pushpankark]

In jquery.min.js I changed ownerGlobal back to ownerDocument.defaultView

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch

this is both patches combined

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/98e3e1a81859
Replace ownerDocument.defaultView with ownerGlobal in browser/. r=dao

Comment 13

[Dão Gottwald [:dao]]

https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=98e3e1a81859

No test failures related to your changes as far as I can see. So I think we're done here.

Thanks :)

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98e3e1a81859

Comment 15

[Saheda Reza [:Antora]]

I have successfully reproduced this on Firefox nightly according to(2016-07-14)

Fixing bug is verified on Latest Nightly-- Build ID:(20160908030434), User Agent: Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101 Firefox/51.0


Tested OS-- Windows10 32bit

Comment 16

[Saheda Reza [:Antora]]

(In reply to Saheda Reza [:Antora] from comment #15)
> I have successfully reproduced this on Firefox nightly according
> to(2016-07-14)
> 
> Fixing bug is verified on Latest Developer Edition-- Build ID:(20160909004004),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:50.0) Gecko/20100101 Firefox/50.0
> 
> 
> Tested OS-- Windows10 32bit