611307 - Cannot install a Style when clicking on the 'Install with Stylish' button at given URL.

Status: RESOLVED INVALID

(Core :: XPConnect, defect)

Description

[Gary [:streetwolf52]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101110 Firefox/4.0b8pre
Build Identifier: 20101110171228

Clicking on the 'Install with Stylish' button at the given URL does not bring up the Add-On Stylish.

Reproducible: Always

Steps to Reproduce:
1. Install Stylish https://addons.mozilla.org/en-US/firefox/addon/2108/
2. Go to the given URL and choose any style to install.
3. Press the 'Install with Stylish' button
Actual Results:  
Nothing happens

Expected Results:  
Stylish edit window should pop up. 

Caused by http://hg.mozilla.org/mozilla-central/rev/43e0c9f82545

Comment 1

[Gary [:streetwolf52]]

Could also be this https://bugzilla.mozilla.org/show_bug.cgi?id=605760 since this patch was included in cset 43e0c9f82545

Comment 2

[Gary [:streetwolf52]]

Could also be this https://bugzilla.mozilla.org/show_bug.cgi?id=605760 since this patch was included in cset 43e0c9f82545

Comment 3

[Alice0775 White]

FYI
This problem is very similar to Bug 604476.

If I modified  Stylish-1.0.11.xpi\content\overlay.js as follows, the problem was fone.

 installFromSite: function(event) {

* snip *

-    for (i in links) {
+    for (let i = 0; i < links.length; i++) {
        switch (links[i].rel) {

Comment 4

[Alice0775 White]

s/fone/gone/

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

I want to call this the result of a bug fix. The JS construct for (i in obj) {} iterates over the prototype chain, resulting in iterating the XPConnect functions and properties on the prototype chain (in this case: length, item, and namedItem). bug 604476 fixed XrayWrapper so that we properly do this now. IMO the for loop from 0 to length is safer and the right "fix" for this bug. Alternatively, you could use the Iterator() constructor trick as was used in bug 604476.

For additional fun, try this in a content document and you'll see that XrayWrappers now more closely map to the behavior of the underlying object.

Comment 6

[Brendan Eich [:brendan]]

Blake, just checking: JS core class prototypes never have enumerable properties. Are we making proto-props enumerable where we should not (i.e., where other DOM implementations do not)?

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #6)
> Blake, just checking: JS core class prototypes never have enumerable
> properties. Are we making proto-props enumerable where we should not (i.e.,
> where other DOM implementations do not)?

Brendan, XPConnect/DOM properties have always been enumerable. At the very least, both Safari and Chrome agree with us here.

Comment 8

[Brendan Eich [:brendan]]

Thanks -- not great but we are stuck with enumerable proto-props. Consistency, what's that?

/be