After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693814 - Looking glass: Extensions tab: buttons closing lg
Looking glass: Extensions tab: buttons closing lg
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-14 15:35 UTC by Alban Crequy
Modified: 2013-02-20 23:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] lg: add a reference to lookingGlass in the Extensions tab. (1.13 KB, patch)
2013-02-14 15:35 UTC, Alban Crequy
needs-work Details | Review
[PATCH] lg: add a reference to lookingGlass in the Extensions tab. (1.66 KB, patch)
2013-02-20 18:59 UTC, Alban Crequy
committed Details | Review

Description Alban Crequy 2013-02-14 15:35:16 UTC
Created attachment 236086 [details] [review]
[PATCH] lg: add a reference to lookingGlass in the Extensions tab.

[PATCH] lg: add a reference to lookingGlass in the Extensions tab.

When the user clicks on "View Source" or "Web Page" in the "Extensions" tab of
looking glass, the callback _onViewSource() or _onWebPage() is called and they
try to close looking glass: this._lookingGlass.close();

But it does not work and generate the exception "this._lookingGlass is
undefined". This patch fixes that.
Comment 1 Florian Müllner 2013-02-14 15:46:05 UTC
Review of attachment 236086 [details] [review]:

::: js/ui/lookingGlass.js
@@ +915,2 @@
         this._extensions = new Extensions();
+        this._extensions._lookingGlass = this;

No. You should never access (not to mention *write*) a private property from the outside. Instead, do what all the other tabs do:

(1) Add a "lookingGlass" parameter to the Extensions class
(2) Do "this._lookingGlass = lookingGlass;" in Extensions' _init()
(3) Call it like "this._extensions = new Extensions(this);"
Comment 2 Florian Müllner 2013-02-14 15:46:08 UTC
Review of attachment 236086 [details] [review]:

::: js/ui/lookingGlass.js
@@ +915,2 @@
         this._extensions = new Extensions();
+        this._extensions._lookingGlass = this;

No. You should never access (not to mention *write*) a private property from the outside. Instead, do what all the other tabs do:

(1) Add a "lookingGlass" parameter to the Extensions class
(2) Do "this._lookingGlass = lookingGlass;" in Extensions' _init()
(3) Call it like "this._extensions = new Extensions(this);"
Comment 3 Alban Crequy 2013-02-20 18:59:37 UTC
Created attachment 236972 [details] [review]
[PATCH] lg: add a reference to lookingGlass in the Extensions tab.

Thanks for the review. Patch updated with:
- pass lookingGlass through Extensions' constructor
- add bug URL in the commit log
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-02-20 19:04:25 UTC
Review of attachment 236972 [details] [review]:

This is correct.
Comment 5 Alban Crequy 2013-02-20 23:16:24 UTC
Comment on attachment 236972 [details] [review]
[PATCH] lg: add a reference to lookingGlass in the Extensions tab.

Committed on master as a8a4a85daccf35a02afcc232735cb046e55b6ab9