Open Bug 684235 Opened 9 years ago Updated 8 years ago

Use documentURIObject in the check for shouldLoadFavIcon so that we do the right thing with about:-style error pages. See Bug 453442.

Categories

(SeaMonkey :: Tabbed Browser, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: philip.chee, Unassigned)

Details

(Keywords: helpwanted)

Attachments

(1 file)

Over time various parts of Firefox Bug 453442 (Secure favicon.ico request pop-ups certificate error dialog) have been ported over. This bug ports the remaining bit in tabbrowser.xml.
(In reply to Philip Chee from comment #0)
> This bug ports the remaining bit
I don't think any of our favicon code uses documentURIObject, or what did you mean by the remaining bit?
There are three parts in Firefox patch https://bug453442.bugzilla.mozilla.org/attachment.cgi?id=344145. Two of the three parts have been ported over as part of other bugs. This patch implements the remaining bit that never got copied.

This is the comment that accompanies the Firefox patch:

> -            else if (this.shouldLoadFavIcon(browser.currentURI)) {
> +            // Use documentURIObject in the check for shouldLoadFavIcon so that we
> +            // do the right thing with about:-style error pages.  Bug 453442
> +            else if (this.shouldLoadFavIcon(browser.contentDocument.documentURIObject)) {
Comment on attachment 557829 [details] [diff] [review]
Patch v1.0 s/currentURI/contentDocument.documentURIObject/

Cancelling review request.

<NeilAway>RattyAway: none of our other callers uses documentURIObject
<RattyAway>NeilAway: two callers use documentURIObject
<NeilAway>RattyAway: actually I was thinking of SetPageProxyState, which is where we actually load favicons
<RattyAway>NeilAway: so you think we don't have the problem reported in Bug 453442?
<NeilAway>RattyAway: oh, I'm sure we do, just that your patch won't fix it
<RattyAway>NeilAway: so I should fix it in SetPageProxyState?
<NeilAway>RattyAway: well, something else needs fixing, yes
<RattyAway>NeilAway: sorry I don't know what to do then.
<NeilAway>wasn't suggesting you would
Attachment #557829 - Flags: review?(neil)
Summary: [tabbrowser] Use contentDocument.documentURIObject for shouldLoadFavIcon()/loadFavIcon(). → Use documentURIObject in the check for shouldLoadFavIcon so that we do the right thing with about:-style error pages. See Bug 453442.
Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
Keywords: helpwanted
You need to log in before you can comment on or make changes to this bug.