589598 - InstallTrigger is not defined for frames inside webpages

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Gary King]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre Firefox/3.6.8
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre Firefox/3.6.8

Open up menu/add ons/browse all add ons/select extension. Click on extension to be installed and nothing happens

Reproducible: Always

Steps to Reproduce:
1.Menu
2.add ons
3.click to install (see above)
Actual Results:  
Nothing

Expected Results:  
I would expect the add on to install.

install the add on and restart.

Comment 1

[Scoobidiver (away)]

For the moment, a lot of add-ons are not enabled for b5pre, which means "add to Firefox" button is white and not green.

Please, be more specific. Which b5pre compatible add-on does not install ?

Comment 2

[Gary King]

Any Addon will not install one in particular is Noscript. And no the install buttons are green. See attached link.

http://i20.photobucket.com/albums/b243/TK-1913/ns.png

When I click on the Green Install Button nothing happens. But in my investigation I have found that if I use a link to the add on site such as https://addons.mozilla.org/en-US/firefox/search/?q=&cat=all&lver=any&pid=1&sort=&pp=20&lup=&advanced=

It will allow me to install an add on. But when I go through the steps described above nothing happens. I have tried this with a clean profile and the latest hourly build with the same results.

Comment 3

[Matthias Versen [:Matti]]

Please set your UA string to the default value and try it again.

Comment 4

[Gary King]

To comment 3. I did that but all it accomplished was to change the add ons that were not compatible to white and Noscript or any green one would still not install. Doing this also messed up my ability to use Windows Live Mail. As I have stated I tried this on a fresh profile, a fresh profile using a hourly build with the same results. I can install anything by using a link to the add ons but not using the standard methods. I do have the latest version of ACR installed as well.

Comment 5

[Scoobidiver (away)]

It works for me.

Please, go to "Options > Security > Warn me when sites try to install add-ons > Exceptions". Check that "addons.mozilla.org" is in "Allow" status

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

You know that you will have to override the compatibility checks? If you do not wanna do it manually install the Add-ons Compatibility Reporter first. Only with compatibility checks disabled you can install not compatible add-ons.

Comment 7

[Matthias Versen [:Matti]]

Isn't there a warning that reports "incompatible addon" ?

Comment 8

[Gary King]

To comment 5.it is allowed. To comment 6 ACR was installed. to comment 7, yes there is. I cannot install any add on period.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Which add-ons are you trying to install?

Comment 10

[Gary King]

To comment 9 please see comment 2-Noscript

Comment 11

[Gary King]

OK since I do not seem to be getting anywhere with this I tried several things. I can get it to install if and only if I RC on the green install and open it in a new tab. I did not have to do this prior to this build. There are way too many steps to go through as it is. Any help ot fix or advice would be appreciated.

Comment 12

[Dave Townsend [:mossop]]

What does the error console say after you try to install something?

Comment 13

[Gary King]

I took a ss of it see the link:

http://i20.photobucket.com/albums/b243/TK-1913/ec.png

Comment 14

[Dave Townsend [:mossop]]

Please clear the console and try again, then show what the bottom-most messages are

Comment 15

[Gary King]

The First link is the bottom:

http://i20.photobucket.com/albums/b243/TK-1913/EC3.png

Top:

http://i20.photobucket.com/albums/b243/TK-1913/EC1.png

I hope this helps.

Comment 16

[Dave Townsend [:mossop]]

Try enabling extensions.logging.enabled in about:config and doing the same. Could you also confirm the exact steps you are taking to see this bug, the steps in the first comment don't really make much sense.

Comment 17

[Gary King]

Dis as instructed in comment 16.

Bottom Error list:

http://i20.photobucket.com/albums/b243/TK-1913/errorbottom.png

Top:

http://i20.photobucket.com/albums/b243/TK-1913/errortop.png
To Reproduce:

1. Click on App Menu Button
2. Click on Add ons
3. Click on Extension
4. Click on Get Add ons
5. Click on Top Addons
6. Click Green Install Button for Noscript
7. Nothing happens.

Comment 18

[Dave Townsend [:mossop]]

Ok that makes more sense. This is a regression from bug 550936. InstallTrigger is undefined in frames in webpages, I get this error:

Error: InstallTrigger is not defined
Source File: https://addons.mozilla.org/media//js/common-min.js?build=060c88b
Line: 16

Comment 19

[Gary King]

Thanks Dave, How do I fix this and are others users getting this as well? I used to get sort of a gray pop up telling me to restart after I installed an add on. I have no idea of how long this has been doing this as I do not install that many add ons. I can install say TMP from their website without problems. I am sorry I was not more clear about it..

Comment 20

[Dave Townsend [:mossop]]

(In reply to comment #19)
> Thanks Dave, How do I fix this and are others users getting this as well? I
> used to get sort of a gray pop up telling me to restart after I installed an
> add on. I have no idea of how long this has been doing this as I do not install
> that many add ons. I can install say TMP from their website without problems. I
> am sorry I was not more clear about it..

You should visit addons.mozilla.org directly rather than through the add-ons manager.

Comment 21

[Gary King]

This is just a temporary work around then as I knew I could do that. It works properly in Beta 3 and the Pre Beta 4 candidates.

Comment 22

[Dave Townsend [:mossop]]

Yes, the patch that broke this landed just after beta 4.

Comment 23

[Gary King]

Thanks for your help Dave. I appreciate it!

Comment 24

[Benjamin Smedberg]

Is this frames inside *any* web page, or only frames in the addons manager?

Comment 25

[Dave Townsend [:mossop]]

Any webpage, see the url for a simple example.

Comment 26

[Dave Townsend [:mossop]]

Alon, is this something you'd be able to figure out?

Comment 27

[Alon Zakai (:azakai)]

(In reply to comment #26)
> Alon, is this something you'd be able to figure out?

I'll look into this.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #20)
> You should visit addons.mozilla.org directly rather than through the add-ons
> manager.

This also doesn't work at least for installing themes like Walnut from the search results page on AMO:

https://addons.mozilla.org/en-US/firefox/search/?q=walnut&cat=all&lver=any&pid=1&sort=&pp=20&lup=&advanced=

Clicking "Install Anyway" also brings up the "InstallTrigger is not defined" error.

But opening the add-ons page directly let me install the theme:

https://addons.mozilla.org/en-US/firefox/addon/122/

Comment 29

[Alon Zakai (:azakai)]

Attachment: Patch

Apparently there is some subtle difference in how events work for iframes. This patch seems to fix it, but I don't have a good explanation why.

Waiting on try server to see if this breaks anything, hopefully all is well.

Comment 30

[Alon Zakai (:azakai)]

Comment on attachment 469296 [details] [diff] [review]
Patch

Patch looks good on try server.

Comment 31

[Dave Townsend [:mossop]]

Comment on attachment 469296 [details] [diff] [review]
Patch

Can you add an automated test for this please?

Comment 32

[Alon Zakai (:azakai)]

Attachment: Patch v2

Patch with tests.

Comment 33

[Dave Townsend [:mossop]]

Comment on attachment 469644 [details] [diff] [review]
Patch v2

Please use the existing browser-chrome harness and ensure that calling InstallTrigger.install works from the inner-frame and that it receives callbacks correctly.

Comment 35

[Alon Zakai (:azakai)]

Attachment: Patch v3

Done as requested.

Comment 36

[Dave Townsend [:mossop]]

Comment on attachment 470472 [details] [diff] [review]
Patch v3

Awesome.

You could also make the iframe default to about:blank and then just load the correct url in prepChild and not have to add the check in installtrigger.html but I guess this is fine either way.

Comment 37

[Alon Zakai (:azakai)]

Attachment: Patch v4

Whoops, forgot the Makefile changes. Fixed here.

Also I made it use about:blank as requested, that is a much nicer way to do it.

Comment 39

[Dave Townsend [:mossop]]

landed: http://hg.mozilla.org/mozilla-central/rev/be48a89b9561

Comment 40

[Gary King]

Thanks!

Comment 41

[Dave Townsend [:mossop]]

Backed out to investigate the perf impact of it

Comment 42

[Gary King]

This is just great. I have been waiting all day for a build and now you back out the Bug fix I was waiting for. Was this not already tested see comments 30, 31, 32, 35, and 37.

Comment 43

[Dave Townsend [:mossop]]

(In reply to comment #42)
> This is just great. I have been waiting all day for a build and now you back
> out the Bug fix I was waiting for. Was this not already tested see comments 30,
> 31, 32, 35, and 37.

Today's nightly should have the fix for you to test. The try server is never a great source of performance numbers and while I feel it is very unlikely that this patch is actually the cause of the performance regression there is still a performance regression that happened at the same time as this patch was landed. Backing it out will confirm things one way or another.

Comment 44

[Gary King]

FWIW this fix does work and I am NOT seeing any speed degradation.

Comment 45

[Dave Townsend [:mossop]]

(In reply to comment #44)
> FWIW this fix does work and I am NOT seeing any speed degradation.

It is unlikely that any of the performance impact of this patch would be visible to the naked eye but the backout seems to have confirmed that this caused a bunch of degredations in the talos test suites. These need to be understood before we can take this fix.

http://bit.ly/cY9L3n

A couple of ways I think we could improve the speed and memory usage of this code:

We could switch to using a lazy getter to add InstallTrigger to the windows.
We could not add the callback listener until the first use of InstallTrigger.install.
We might be able to get away with just a single instance of InstallTrigger for multiple webpages in the process, that would require some care though.

Comment 46

[Alon Zakai (:azakai)]

I have been investigating this today. So far I cannot reproduce a Tp4 RSS increase with the patch, but I am trying a rebuild from scratch now to be sure.

I have also been testing ways to reduce memory usage, basically the ones just mentioned:

1. The lazy getter approach doesn't work - all items on windows are accessed, apparently as part of a security check (this has to do with the toSource workaround we also need). So the getter is always called anyhow.

2. Sharing a single installtrigger would be hard, as they need to remember source urls and such. They can share the list of callbacks, though. However, Tp4 always has an empty list of callbacks (no installs are done) - so it seems extremely unlikely that a few []s can lead to a 12%(!) RSS increase.

In fact in general I cannot see how the objects created here can lead to so much memory being used.

Comment 47

[Alon Zakai (:azakai)]

I finished another round of local Tp4 tests - still cannot reproduce the suspected perf problems.

Comment 48

[Dave Townsend [:mossop]]

(In reply to comment #29)
> Created attachment 469296 [details] [diff] [review]
> Patch
> 
> Apparently there is some subtle difference in how events work for iframes. This
> patch seems to fix it, but I don't have a good explanation why.

aEvent.target is the document that sent the DOMWindowCreated event. aEvent.target.defaultView is the window for that document and so what InstallTrigger should get attached to. window.content for webpages is basically the same as window.top so for inner frames points to the outermost page so it was always trying to hook up to the main window.

I don't think that the originalTarget change actually makes any difference here but I could be wrong.

(In reply to comment #47)
> I finished another round of local Tp4 tests - still cannot reproduce the
> suspected perf problems.

Are you using the same pageset as the full talos runs? My best guess here would be that if the tp4 pageset contains many pages with frames then we would see a regression from this patch because there would be that many more InstallTriggers created and hooked up to the webpage.

Comment 49

[Alon Zakai (:azakai)]

(In reply to comment #48)
> (In reply to comment #47)
> > I finished another round of local Tp4 tests - still cannot reproduce the
> > suspected perf problems.
> 
> Are you using the same pageset as the full talos runs? My best guess here would
> be that if the tp4 pageset contains many pages with frames then we would see a
> regression from this patch because there would be that many more
> InstallTriggers created and hooked up to the webpage.

I was using the full Talos pageset. But as discussed on irc, it seems I could not reproduce it locally since it is noticeable everywhere *but* linux32 (and I am on linux32).

I guess I'll install linux64 (seems the clearest effect on that) and continue there.

Comment 50

[Alon Zakai (:azakai)]

Attachment: Patch v5

I still can't reproduce this locally, even on linux64. Odd. Must be something special on Talos that makes it sensitive to the issue. Anyhow, here is a patch using getters (so it avoids holding on to references completely) that appears to work fine according to Talos:

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=c3e21097bb78&newRev=2cce6fd2c51d&tests=a11y,tdhtml,tdhtml_nochrome,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tgfx,tgfx_nochrome,tscroll,tsvg,tsvg_opacity,ts,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen&submit=true

Issues:

* Setting the getter sometimes raises an error. As mentioned in the patch, this doesn't prevent it from working, but isn't caused by any of the reasons I would suspect. I will investigate more, but I'm posting the patch meanwhile since beta6 is around the corner.
* As also mentioned in the patch, a downside to using getters this way is that window.InstallTrigger !== window.InstallTrigger, as a new one is created each time. But maybe that is acceptable?

Comment 51

[Dave Townsend [:mossop]]

(In reply to comment #50)
> * As also mentioned in the patch, a downside to using getters this way is that
> window.InstallTrigger !== window.InstallTrigger, as a new one is created each
> time. But maybe that is acceptable?

You could solve that by using a smart getter that deletes the getter and replaces it with the new InstallTrigger when called.

Comment 52

[Alon Zakai (:azakai)]

(In reply to comment #51)
> (In reply to comment #50)
> > * As also mentioned in the patch, a downside to using getters this way is that
> > window.InstallTrigger !== window.InstallTrigger, as a new one is created each
> > time. But maybe that is acceptable?
> 
> You could solve that by using a smart getter that deletes the getter and
> replaces it with the new InstallTrigger when called.

In other words, to cache an InstallTrigger object? That would mean holding a reference to it. One might expect this caching to only happen when the InstallTrigger is actually used, which is very rare, so holding a ref in that case wouldn't be bad. But all properties on all windows are actually read anyhow, so the getter *will* end up getting called, and each window will have an InstallTrigger object, bringing us right back to where we started.

Comment 53

[Dave Townsend [:mossop]]

No, I mean do the following:

win.__defineGetter__("InstallTrigger", function() {
  delete win.InstallTrigger;
  win.InstallTrigger = new InstallTrigger(); // Or whatever is needed
  return win.InstallTrigger;
});

Comment 54

[Benjamin Smedberg]

"But all properties on all windows are actually read anyhow". By what code? This sounds like a bad thing in general. And Dave, that would negate the memoizer. We might still be causing leaks or delayed cycles with that code, if win.InstallTrigger holds the window alive.

Comment 55

[Alon Zakai (:azakai)]

(In reply to comment #54)
> "But all properties on all windows are actually read anyhow". By what code?
> This sounds like a bad thing in general. And Dave, that would negate the
> memoizer.

Yes, that was what I meant - memoization won't work because of that problem. The result is the same as just placing an InstallTrigger on every window.

I will look into what code is reading all properties on windows. However, if the only price of the current patch is that window.InstallTrigger !== window.InstallTrigger, maybe it doesn't make sense to wait for figuring out what the other issue is.

Comment 56

[Dave Townsend [:mossop]]

(In reply to comment #54)
> "But all properties on all windows are actually read anyhow". By what code?
> This sounds like a bad thing in general. And Dave, that would negate the
> memoizer. We might still be causing leaks or delayed cycles with that code, if
> win.InstallTrigger holds the window alive.

Oh right yeah, that seems to be a problem we should resolve. However we could improve matters by listening to the window's unload event and removing its InstallTrigger reference then.

But I guess if we can't figure out why the platform is doing stupid things then this doesn't cost us much.

Comment 57

[Alon Zakai (:azakai)]

Hmm, I can't reproduce the problem of the getter being called needlessly. Perhaps it only happens in one kind of build - I can't remember which I noticed the problem on before. To be sure I pushed to try with a memoizing version, we'll see how it goes.

Comment 58

[Alon Zakai (:azakai)]

Attachment: Patch v6

Ok, this memoizing version passes all try tests, and Talos looks fixed too (except for shutdown, but I am guessing that is just noise),

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=c3e21097bb78&newRev=03cea36b3209&tests=a11y,tdhtml,tdhtml_nochrome,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tgfx,tgfx_nochrome,tscroll,tsvg,tsvg_opacity,ts,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen&submit=true

So it seems there is no problem with the memoizing approach - the odd behavior I saw before was probably something weird in how I was testing.

Comment 59

[Alon Zakai (:azakai)]

Attachment: Patch v7

Oops, fixed a this/self mistake.

Comment 60

[Dave Townsend [:mossop]]

Comment on attachment 473104 [details] [diff] [review]
Patch v7

Minusing only because of the unknown failure case

>diff --git a/toolkit/mozapps/extensions/content/extensions-content.js b/toolkit/mozapps/extensions/content/extensions-content.js

>+    // For unknown reasons this raises an error sometimes. This is not
>+    // because there already is a getter there. It is also not because
>+    // we mistakenly try to place one on chrome. In any case it does not
>+    // prevent this from working.
>+    try {
>+      window.wrappedJSObject.__defineGetter__("InstallTrigger", this.getter);
>+    }
>+    catch(e) {
>+      dump("Ignored InstallTrigger setup error\n");
>+    }
>+  },

Can we narrow down why this fails and get a bug on file for it, I don't really want this to land with such an obvious unknown.

>+  getter: function() {

Call this function createInstallTrigger.

Comment 61

[Alon Zakai (:azakai)]

Attachment: Patch v8

Ok, I spent some more time investigating that issue. Looks like the patch wasn't identifying chrome windows properly - the problem is not just actual ChromeWindows, but also normal windows showing |chrome://| or |about:| pages. Checking for those directly also simplifies the error checking in the patch.

Also fixed the function name as requested.

Patch passes try and rss looks good on Talos,

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=26807152d00d&newRev=0ee71879998e&tests=a11y,tdhtml,tdhtml_nochrome,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tgfx,tgfx_nochrome,tscroll,tsvg,tsvg_opacity,ts,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen&submit=true

Comment 62

[Dave Townsend [:mossop]]

Comment on attachment 473315 [details] [diff] [review]
Patch v8

Please provide patches with more context in future, 8 lines is the recommended number.

>+  getter: function createInstallTrigger() {

I meant the property should be called createInstallTrigger

>-new InstallTriggerManager();
>+var manager = new InstallTriggerManager();

Any reason for this change?

Comment 63

[Alon Zakai (:azakai)]

(In reply to comment #62)
> Comment on attachment 473315 [details] [diff] [review]
> Patch v8
> 
> Please provide patches with more context in future, 8 lines is the recommended
> number.
> 

Sorry about that, I grabbed it directly from my patch queue by mistake.

> >+  getter: function createInstallTrigger() {
> 
> I meant the property should be called createInstallTrigger

Ah, I misunderstood... want a fixed patch?

> 
> >-new InstallTriggerManager();
> >+var manager = new InstallTriggerManager();
> 
> Any reason for this change?

Yes, the created InstallTrigger objects need to call manager.addCallback (the manager stores all the callback info now, simpler and more efficient that way).

Comment 64

[Dave Townsend [:mossop]]

(In reply to comment #63)
> (In reply to comment #62)
> > Comment on attachment 473315 [details] [diff] [review] [details]
> > Patch v8
> > 
> > Please provide patches with more context in future, 8 lines is the recommended
> > number.
> > 
> 
> Sorry about that, I grabbed it directly from my patch queue by mistake.
> 
> > >+  getter: function createInstallTrigger() {
> > 
> > I meant the property should be called createInstallTrigger
> 
> Ah, I misunderstood... want a fixed patch?

Just make the change and carry over the review.

> > >-new InstallTriggerManager();
> > >+var manager = new InstallTriggerManager();
> > 
> > Any reason for this change?
> 
> Yes, the created InstallTrigger objects need to call manager.addCallback (the
> manager stores all the callback info now, simpler and more efficient that way).

Missed that, ok that's fine

Comment 65

[Alon Zakai (:azakai)]

Attachment: Patch v8.1

Fixed function name, carried r+.

Comment 66

[Alon Zakai (:azakai)]

Attachment: PATCH FOR CHECKIN

Patch for checkin.

Comment 67

[Gary King]

Good maybe this can land soon

Comment 68

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/1bd1a125cad9

Comment 69

[Gary King]

This is NOT fixed. See attachment.

[IMG]http://i20.photobucket.com/albums/b243/TK-1913/getaddon.png[/IMG]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910 Firefox/4.0b6pre

Comment 70

[Gary King]

Please Reopen

Comment 71

[u88484]

Not sure if this broke it but nothing shows up in the add-ons manager when the Get Add-ons is clicked.  I just get a blank blue background where the page should show.

hourly build Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910 Firefox/4.0b6pre from cset cca361001fda

Comment 72

[Gary King]

If nothing shows up, it is Broken, what is below is suppose to show up.

https://addons.mozilla.org/en-US/firefox/search/?q=&cat=all&lver=4.0&pid=5&sort=name&pp=50&lup=3+months+ago&advanced=on

How does one add an extension from a blank gray page??

Comment 73

[Gary [:streetwolf52]]

Normally all I get is a grey screen.  If I refresh the screen I am sent here:

chrome://mozapps/content/extensions/extensions.xul


Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910 Firefox/4.0b6pre - Build ID: 20100910041829

Comment 74

[Gary [:streetwolf52]]

(In reply to comment #73)
> Normally all I get is a grey screen.  If I refresh the screen I am sent here:
> 
> chrome://mozapps/content/extensions/extensions.xul
> 
> 
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910
> Firefox/4.0b6pre - Build ID: 20100910041829

Actually I'm really sent here: http://www.mozilla.org/

Comment 75

[Gary King]

I am as well but that is not where we need to go. That is still not correct. This needs to be re opened.

Comment 76

[Gary [:streetwolf52]]

(In reply to comment #75)
> I am as well but that is not where we need to go. That is still not correct.
> This needs to be re opened.

Oh, I agree.

Comment 77

[Dave Townsend [:mossop]]

(In reply to comment #69)
> This is NOT fixed. See attachment.
> 
> [IMG]http://i20.photobucket.com/albums/b243/TK-1913/getaddon.png[/IMG]
> 
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910
> Firefox/4.0b6pre

That is bug 593387

(In reply to comment #74)
> (In reply to comment #73)
> > Normally all I get is a grey screen.  If I refresh the screen I am sent here:
> > 
> > chrome://mozapps/content/extensions/extensions.xul
> > 
> > 
> > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910
> > Firefox/4.0b6pre - Build ID: 20100910041829
> 
> Actually I'm really sent here: http://www.mozilla.org/

That is bug 584693

This bug remains fixed and did not cause either of the things you are talking about.

Comment 78

[Gary King]

http://i20.photobucket.com/albums/b243/TK-1913/getaddon.png

The above is what I get when I click on Get Addons , I do not get http://www.mozilla.org/ until I refresh the page and this is NOT on a new profile so as far as I am concerned this bug is NOT fixed.

Comment 79

[Dave Townsend [:mossop]]

(In reply to comment #78)
> http://i20.photobucket.com/albums/b243/TK-1913/getaddon.png
> 
> The above is what I get when I click on Get Addons , I do not get
> http://www.mozilla.org/ until I refresh the page and this is NOT on a new
> profile so as far as I am concerned this bug is NOT fixed.

As I said, that is bug 593387(In reply to comment #78)
> http://i20.photobucket.com/albums/b243/TK-1913/getaddon.png
> 
> The above is what I get when I click on Get Addons , I do not get
> http://www.mozilla.org/ until I refresh the page and this is NOT on a new
> profile so as far as I am concerned this bug is NOT fixed.

The fact that the get add-ons content does not appear has nothing to do with this bug which is about whether installs from there work. Since you aren't testing that you can't say whether the bug is fixed or not.

Comment 80

[Gary King]

How can I when all I get is a gray screen?? The problem needs to be fixed one way or another. There are many users that do not even like the new AOM and all of the space it takes up on the screen and the fact that it does not function makes it worse. Maybe it is time to go back to the former stable version of getting addons??

Comment 81

[Dave Townsend [:mossop]]

(In reply to comment #80)
> How can I when all I get is a gray screen??

I've added a new testcase to the URL that works without AMO.

> The problem needs to be fixed one way or another.

And this problem with InstallTrigger not working inside frames is fixed. I pointed you to the problem you are seeing with a blank get add-ons bug above, bug 593387. It is important that we keep separate issues in separate bug reports or the reports become unmanageable and impossible to work on.

> There are many users that do not even like the new AOM and all
> of the space it takes up on the screen and the fact that it does not function
> makes it worse. Maybe it is time to go back to the former stable version of
> getting addons??

They and you are of course welcome to go back to the Firefox 3.6 release builds with the old add-ons manager if you do not feel you can put up with the issues present in nightly builds.

Comment 82

[Gary King]

I understand and I will stay with the nightlies. This problem was fixed once before and then backed out. I know that you will get it right before it goes final. It really is not that big of an issue as I don't instal extensions every day. Sorry for the attitude.

Comment 83

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101204 Firefox/4.0b8pre ID:20101204030328