Closed
Bug 304727
Opened 19 years ago
Closed 19 years ago
Attempting to install an extension no longer triggers yellow info bar (notification) if a site isn't whitelisted or if software installation is disabled
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox1.5
People
(Reporter: ria.klaassen, Assigned: Gavin)
References
()
Details
(Keywords: fixed1.8, regression)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
mconnor
:
review+
shaver
:
approval1.8b4+
|
Details | Diff | Splinter Review |
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•19 years ago
|
||
Dup of bug 303058?
Assignee | ||
Comment 2•19 years ago
|
||
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•19 years ago
|
||
Between 2005081112 (works) and 2005081121 (fails).
Updated•19 years ago
|
Keywords: regression
Assignee | ||
Comment 4•19 years ago
|
||
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
Assignee | ||
Updated•19 years ago
|
Assignee | ||
Comment 5•19 years ago
|
||
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
Assignee | ||
Comment 6•19 years ago
|
||
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)
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•19 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•19 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•19 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•19 years ago
|
Flags: blocking1.8b4? → blocking1.8b4+
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•19 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•19 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 (:
Assignee | ||
Comment 10•19 years ago
|
||
(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.
Assignee | ||
Updated•19 years ago
|
Attachment #192766 -
Attachment is obsolete: true
Attachment #192766 -
Flags: review?(mconnor)
Assignee | ||
Comment 11•19 years ago
|
||
This fixes it, but isn't very pretty. Backing out the changes made by the
feedview landing (bug 305134) is probably a better bet.
Comment 12•19 years ago
|
||
*** Bug 305648 has been marked as a duplicate of this bug. ***
Updated•19 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.
Assignee | ||
Comment 14•19 years ago
|
||
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
Assignee | ||
Comment 15•19 years ago
|
||
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•19 years ago
|
Attachment #193812 -
Flags: review?(mconnor) → review+
Comment 16•19 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;
},
Assignee | ||
Comment 17•19 years ago
|
||
(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?
Assignee | ||
Updated•19 years ago
|
Attachment #193812 -
Flags: approval1.8b4?
Comment 18•19 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?
Assignee | ||
Comment 19•19 years ago
|
||
(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•19 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)?
Assignee | ||
Comment 21•19 years ago
|
||
(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•19 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•19 years ago
|
||
(In reply to comment #23)
Oh great, that is caused by some extension I had installed (:
Assignee | ||
Comment 25•19 years ago
|
||
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
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: fixed1.8
Comment 26•19 years ago
|
||
*** Bug 305965 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•