InstallTrigger is not defined for frames inside webpages

VERIFIED FIXED in mozilla2.0b7

Status

()

Toolkit
Add-ons Manager
--
major
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Gary King, Assigned: azakai)

Tracking

({regression})

Trunk
mozilla2.0b7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(URL)

Attachments

(2 attachments, 8 obsolete attachments)

(Reporter)

Description

7 years ago
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

7 years ago
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 ?
(Reporter)

Comment 2

7 years ago
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.
Please set your UA string to the default value and try it again.
(Reporter)

Comment 4

7 years ago
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.
Component: General → Add-ons Manager
Product: Firefox → Toolkit
QA Contact: general → add-ons.manager

Comment 5

7 years ago
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
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.
Isn't there a warning that reports "incompatible addon" ?
(Reporter)

Comment 8

7 years ago
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.
Which add-ons are you trying to install?
(Reporter)

Comment 10

7 years ago
To comment 9 please see comment 2-Noscript
(Reporter)

Comment 11

7 years ago
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.
What does the error console say after you try to install something?
(Reporter)

Comment 13

7 years ago
I took a ss of it see the link:

http://i20.photobucket.com/albums/b243/TK-1913/ec.png
Please clear the console and try again, then show what the bottom-most messages are
(Reporter)

Comment 15

7 years ago
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.
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.
(Reporter)

Comment 17

7 years ago
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.
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
Blocks: 550936
Status: UNCONFIRMED → NEW
blocking2.0: --- → betaN+
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Summary: Cannot install an extension using Get Add ons → InstallTrigger
(Reporter)

Comment 19

7 years ago
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..
(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.
Summary: InstallTrigger → InstallTrigger is not defined for frames inside webpages
(Reporter)

Comment 21

7 years ago
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.
Yes, the patch that broke this landed just after beta 4.
(Reporter)

Comment 23

7 years ago
Thanks for your help Dave. I appreciate it!
Keywords: regression
Version: unspecified → Trunk
Is this frames inside *any* web page, or only frames in the addons manager?
Any webpage, see the url for a simple example.
Alon, is this something you'd be able to figure out?
Assignee: nobody → azakai
(Assignee)

Comment 27

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

I'll look into this.
(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/
(Assignee)

Comment 29

7 years ago
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.

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

Comment 30

7 years ago
Comment on attachment 469296 [details] [diff] [review]
Patch

Patch looks good on try server.
Attachment #469296 - Flags: review?(dtownsend)
Comment on attachment 469296 [details] [diff] [review]
Patch

Can you add an automated test for this please?
(Assignee)

Comment 32

7 years ago
Created attachment 469644 [details] [diff] [review]
Patch v2

Patch with tests.
Attachment #469296 - Attachment is obsolete: true
Attachment #469644 - Flags: review?(dtownsend)
Attachment #469296 - Flags: review?(dtownsend)
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.
Attachment #469644 - Flags: review?(dtownsend) → review-

Updated

7 years ago
Duplicate of this bug: 591761
(Assignee)

Comment 35

7 years ago
Created attachment 470472 [details] [diff] [review]
Patch v3

Done as requested.
Attachment #469644 - Attachment is obsolete: true
Attachment #470472 - Flags: review?(dtownsend)
Blocks: 591751
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.
Attachment #470472 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 37

7 years ago
Created attachment 470575 [details] [diff] [review]
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.
Attachment #470472 - Attachment is obsolete: true
Attachment #470575 - Flags: review?(dtownsend)
Attachment #470575 - Flags: review?(dtownsend) → review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed

Updated

7 years ago
Duplicate of this bug: 592761
landed: http://hg.mozilla.org/mozilla-central/rev/be48a89b9561
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
(Reporter)

Comment 40

7 years ago
Thanks!
Keywords: checkin-needed
Depends on: 592947
Backed out to investigate the perf impact of it
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 42

7 years ago
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.
(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.
(Reporter)

Comment 44

7 years ago
FWIW this fix does work and I am NOT seeing any speed degradation.
(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.
(Assignee)

Comment 46

7 years ago
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.
(Assignee)

Comment 47

7 years ago
I finished another round of local Tp4 tests - still cannot reproduce the suspected perf problems.
(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.
(Assignee)

Comment 49

7 years ago
(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.
(Assignee)

Comment 50

7 years ago
Created attachment 472656 [details] [diff] [review]
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?
Attachment #470575 - Attachment is obsolete: true
Attachment #472656 - Flags: review?(dtownsend)
(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.
(Assignee)

Comment 52

7 years ago
(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.
No, I mean do the following:

win.__defineGetter__("InstallTrigger", function() {
  delete win.InstallTrigger;
  win.InstallTrigger = new InstallTrigger(); // Or whatever is needed
  return win.InstallTrigger;
});
"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.
(Assignee)

Comment 55

7 years ago
(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.
(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.
(Assignee)

Comment 57

7 years ago
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.
(Assignee)

Comment 58

7 years ago
Created attachment 473101 [details] [diff] [review]
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.
Attachment #472656 - Attachment is obsolete: true
Attachment #473101 - Flags: review?(dtownsend)
Attachment #472656 - Flags: review?(dtownsend)
(Assignee)

Comment 59

7 years ago
Created attachment 473104 [details] [diff] [review]
Patch v7

Oops, fixed a this/self mistake.
Attachment #473101 - Attachment is obsolete: true
Attachment #473104 - Flags: review?(dtownsend)
Attachment #473101 - Flags: review?(dtownsend)
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.
Attachment #473104 - Flags: review?(dtownsend) → review-
(Assignee)

Comment 61

7 years ago
Created attachment 473315 [details] [diff] [review]
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
Attachment #473104 - Attachment is obsolete: true
Attachment #473315 - Flags: review?(dtownsend)
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?
Attachment #473315 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 63

7 years ago
(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).
(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
(Assignee)

Comment 65

7 years ago
Created attachment 473642 [details] [diff] [review]
Patch v8.1

Fixed function name, carried r+.
Attachment #473315 - Attachment is obsolete: true
Attachment #473642 - Flags: review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Comment 66

7 years ago
Created attachment 473660 [details] [diff] [review]
PATCH FOR CHECKIN

Patch for checkin.
(Assignee)

Updated

7 years ago
Whiteboard: PATCH FOR CHECKIN needs checkin (not other one)
(Reporter)

Comment 67

7 years ago
Good maybe this can land soon
http://hg.mozilla.org/mozilla-central/rev/1bd1a125cad9
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: PATCH FOR CHECKIN needs checkin (not other one)
(Reporter)

Comment 69

7 years ago
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
(Reporter)

Comment 70

7 years ago
Please Reopen

Comment 71

7 years ago
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
(Reporter)

Comment 72

7 years ago
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

7 years ago
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

7 years ago
(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/
(Reporter)

Comment 75

7 years ago
I am as well but that is not where we need to go. That is still not correct. This needs to be re opened.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 76

7 years ago
(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.
(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.
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 78

7 years ago
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.
(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.
(Reporter)

Comment 80

7 years ago
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??
(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.
(Reporter)

Comment 82

7 years ago
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.
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101204 Firefox/4.0b8pre ID:20101204030328
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.