If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Attempting to install an extension no longer triggers yellow info bar (notification) if a site isn't whitelisted or if software installation is disabled

RESOLVED FIXED in Firefox1.5

Status

()

Firefox
General
P1
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Ria Klaassen (not reading all bugmail), Assigned: Gavin)

Tracking

({fixed1.8, regression})

Trunk
Firefox1.5
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8b4 +
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050815 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050815 Firefox/1.0+

Online extension installation does not trigger the yellow notifier bar anymore
if the site is not in the whitelist.
But installation works if the domain is listed under the Allowed Sites.

Reproducible: Always

Steps to Reproduce:
1. Create a new profile with default settings.
2. Go to
http://www.extensionsmirror.nl/index.php?s=f25b7e09f1d6079595b79117a6397c52&showtopic=13
and click on the Install button.
3. An hourglass appears for fraction of a second but further nothing.



Expected Results:  
The notifier bar should appear on the top of the screen.

Comment 1

12 years ago
Dup of bug 303058?
The two are fairly unrelated, and the fix for that bug is specific to the plugin
bar, so I'd say no. Ria, do you have a regression range?
(Reporter)

Comment 3

12 years ago
Between 2005081112 (works) and 2005081121 (fails).

Updated

12 years ago
Keywords: regression
This was broken with Ben's change to findChildShell, due to the checkin for bug
303848.
Assignee: nobody → gavin.sharp
Status: UNCONFIRMED → NEW
Component: Extension/Theme Manager → General
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P1
QA Contact: extension.manager → general
Hardware: PC → All
Summary: Online extension installation does not trigger the yellow notification bar anymore if the site is not in the whitelist → "Blocked popup" and extension installation info-bars (yellow notification bar) no longer appear
Target Milestone: --- → Firefox1.1
Version: unspecified → Trunk
Oops, I was mistaken in thinking this affected the blocked popup notification as
well.
Summary: "Blocked popup" and extension installation info-bars (yellow notification bar) no longer appear → Extension installation info-bars (yellow notification bar) no longer appear
Created attachment 192766 [details] [diff] [review]
patch

Fixes this bug, and a couple strict warnings. This is needed trunk&branch. I've
tested that all the findChildShell callers (popup blocked message, extension
install message, feedview) work as they should with this change.
Attachment #192766 - Flags: review?(mconnor)
Status: NEW → ASSIGNED
Summary: Extension installation info-bars (yellow notification bar) no longer appear → Attempting to install an extension no longer triggers yellow info bar (notification) if a site isn't whitelisted or if software installation is disabled

Updated

12 years ago
Flags: blocking1.8b4?
Summary: Attempting to install an extension no longer triggers yellow info bar (notification) if a site isn't whitelisted or if software installation is disabled → Extension installation info-bars (yellow notification bar) no longer appear

Updated

12 years ago
Summary: Extension installation info-bars (yellow notification bar) no longer appear → Attempting to install an extension no longer triggers yellow info bar (notification) if a site isn't whitelisted or if software installation is disabled

Updated

12 years ago
Flags: blocking1.8b4? → blocking1.8b4+

Comment 7

12 years ago
Same problem with FF 1.0.6.

Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.7.10) Gecko/20050717
Firefox/1.0.6

Comment 8

12 years ago
(In reply to comment #6)
> Created an attachment (id=192766) [edit]
> patch

This patch will fail when you have two identical tabs open, but it works when
you lookup/match the docShell ;)

Comment 9

12 years ago
(In reply to comment #8)
> (In reply to comment #6)
> > Created an attachment (id=192766) [edit] [edit]
> > patch
> 
> This patch will fail when you have two identical tabs open, but it works when
> you lookup/match the docShell ;)

Great, I looked at the wrong patch, sorry for the bug spam (:
(In reply to comment #8)
> This patch will fail when you have two identical tabs open, but it works when
> you lookup/match the docShell ;)

As far as I see, that comment is correct... I tried to take a shortcut without
making it check for a provided document, and that leads to simply checking the
URI, which is wrong.
Attachment #192766 - Attachment is obsolete: true
Attachment #192766 - Flags: review?(mconnor)
Depends on: 305134
Created attachment 193387 [details] [diff] [review]
Patch I don't really like

This fixes it, but isn't very pretty. Backing out the changes made by the
feedview landing (bug 305134) is probably a better bet.
*** Bug 305648 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Flags: blocking1.8b5+
As a datapoint, the backout of feedview didn't seem to repair this bug. Gavin's
on the case, star that he is.
Created attachment 193793 [details] [diff] [review]
patch

This seems to work for all cases, but it's late (early!), so I'm going to test
this again later once I've gotten some sleep.
Attachment #193387 - Attachment is obsolete: true
Created attachment 193812 [details] [diff] [review]
patch 2

This works in my testing. Rather trying to maintain a super findChildShell that
uses three different ways to test docShell equality, I split it into two, one
that compares either a document or a URI (for Live Bookmarks and Popup
blocking, respectively), and the other that just compares docShells directly
(for extension install). This effectively reverts extension install to
pre-Feedview, and modifies the function previously used for "Popup blocking" to
be more general, for use with Live bookmarks.
Attachment #193793 - Attachment is obsolete: true
Attachment #193812 - Flags: review?(mconnor)

Updated

12 years ago
Attachment #193812 - Flags: review?(mconnor) → review+

Comment 16

12 years ago
What about this one?

  _getBrowser: function (aDocShell)
  {
    var tabbrowser = getBrowser();
    var browsers = tabbrowser.browsers;

    for (var i = 0; i < browsers.length; ++i) {
      var browser = tabbrowser.getBrowserAtIndex(i);

      if (this.findChildShell(browser.docShell, aDocShell))
        return browser;
    }
    return null;
  },
(In reply to comment #16)
> What about this one?

What about it? If you're suggesting that I not use the temporary "shell"
variable, that's fine by me. Is there anything else? 
Attachment #193812 - Flags: approval1.8b4?

Comment 18

12 years ago
(In reply to comment #17)
> (In reply to comment #16)
> > What about this one?
> 
> What about it? If you're suggesting that I not use the temporary "shell"
> variable, that's fine by me. Is there anything else? 

Yes, because that makes it a little faster. See also:

browsers.length instead of tabbrowser.browsers.length

Also note that _findChildShell is an exact copy of the function we used for
MultiZilla for over I don't know already, so that should be ok and not fail.

Who is the original developer of this line:

var shellInfo = this._getContentShell(targetDoc, feedURI);

Why did he use feedURI (because he thought that having more feeds on a page
would make any difference perhaps?).

p.s. why are these included:

   aDocShell.QueryInterface(Components.interfaces.nsIWebNavigation);
   aDocShell.QueryInterface(Components.interfaces.nsIInterfaceRequestor);

Why is that?
(In reply to comment #18)
> Who is the original developer of this line:
> var shellInfo = this._getContentShell(targetDoc, feedURI);

Ben Goodger

> Why did he use feedURI (because he thought that having more feeds on a page
> would make any difference perhaps?).

Most likely because the clumsily merged findChildShell took a URI parameter, but
it's redundant, since that URI is just built from document.location.href.

> p.s. why are these included:
>    aDocShell.QueryInterface(Components.interfaces.nsIWebNavigation);
>    aDocShell.QueryInterface(Components.interfaces.nsIInterfaceRequestor);

nsIWebNavigation provides currrentURI and nsIInterfaceRequestor provides
getInterface. Better safe than sorry.

Comment 20

12 years ago
BTW; is there a spec for this? I mean, what about the location bar, history
items and bookmarks? Should that also trigger a BIM (Browser Info Message)?
(In reply to comment #20)
> BTW; is there a spec for this? I mean, what about the location bar, history
> items and bookmarks? Should that also trigger a BIM (Browser Info Message)?

A spec for what? Why would those show browser messages? How is it related to
this bug? :)
Comment on attachment 193812 [details] [diff] [review]
patch 2

a=shaver for branch landing.
Attachment #193812 - Flags: approval1.8b4? → approval1.8b4+

Comment 23

12 years ago
(In reply to comment #21)
> (In reply to comment #20)
> > BTW; is there a spec for this? I mean, what about the location bar, history
> > items and bookmarks? Should that also trigger a BIM (Browser Info Message)?
> 
> A spec for what? 

A spec that describes how this should work, i.e. when a BIM should be displayed.

> Why would those show browser messages? 

Steps to reproduce:

1) copy the following link into your locationbar:
http://downloads.mozdev.org/multizilla/multiviews-v1810.xpi and press enter
2) cancel the installation (no BIM displayed)
3) add the link as bookmark
4) click on the bookmark (no BIM displayed)
5) visit www.mozilla.org
6) go back in history and select the XPInstall link (no BIM displayed)

How is it related to this bug? :)

That seems pretty obvious to me ;)

Comment 24

12 years ago
(In reply to comment #23)

Oh great, that is caused by some extension I had installed (:
Trunk:
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.492; previous revision: 1.491
done

1.8 Branch:
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.479.2.13; previous revision: 1.479.2.12
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: fixed1.8
*** Bug 305965 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Keywords: fixed1.8
Whiteboard: fixed1.8
You need to log in before you can comment on or make changes to this bug.