313961 - iframes appear in the Go menu.

Status: RESOLVED FIXED

(Camino Graveyard :: History, defect)

Description

[Graeme Porteous]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051026 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051026 Camino/1.0+

iframes appear in the Go menu, they don't appear in about:history and are destroyed at the end of the session.

Reproducible: Always

Steps to Reproduce:
1. Load the URL
2. Look at the Go menu

Actual Results:  
See extra menu items linking to iframes.

Expected Results:  
Sould be no extra menu items taking up the precious slots in the Go menu.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

See bug 280390 comment 9 for confirmation of this and a bit of analysis.  We closed as WFM bug 186746 (which seemed to be about them being added to History, which they're not in this case), so I think we can confirm this instead of reopening that one.

Also see bug 280390 comment 10 where I wondered if this bug might be bug 148794 (see bug 148794 comment
49)?

Comment 2

[Simon Fraser [no longer active]]

See also bug 237717.

Comment 3

[Mike Pinkerton (not reading bugmail)]

since bug 237717 was fixed, has this also been fixed?

Comment 4

[Samuel Sidler (old account; do not CC)]

(In reply to comment #3)
> since bug 237717 was fixed, has this also been fixed?
> 

That bug was fixed almost a year ago... something tells me Graeme is using a more recent build than that. ;)

Comment 5

[Graeme Porteous]

Yes confirming, this bug is still present in the 2005-11-01 branch build.

Comment 6

[Simon Fraser [no longer active]]

This has to be core. Is it bug 148794?

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I thought it might be (comment 1).  I'm not seeing it in Fx, though--not in its Go menu, Back button menu, nor in its awful History sidebar.

These are the entries I see in Camino (regardless of whether their contents are actually shown):

http://www.macosxhints.com/bbm_ads/iframe_banner.php?
http://www.macosxhints.com/bbm_ads/iframe_bigbox.php
http://www.macosxhints.com/bbm_ads/iframe_tile.php

Comment 8

[Mike Pinkerton (not reading bugmail)]

on what day did this regress? this is a serious beta blocker and must have been a recent core change to cause it.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Well, based on my comments in bug 280390, I saw it on 2 Aug, so it's not recent.  I have no idea when it appeared, though; there wasn't a testcase or sample URL in bug 186746 when we closed it WFM in December, so maybe it was never fixed and neither Chris C nor I had been browsing the right sites at that time?

Or maybe it really is bug 148794 like Simon and I thought, and Fx has forked that code so that's why it doesn't repro in Fx?

Comment 10

[Graeme Porteous]

This has been broken for a while, ever since the Go menu was fixed in 10.4 - bug 292694. I've only got 10.4 as I can't test any further back :(

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This is interesting.  

Currently, these iframes only appear in the Go menu global history, not the session history on the Back button nor the global history in about:history/Bookmarks Mgr.  That seems odd, as I'd think that Go menu global history = about:history global history.

Based on Graeme's last comment and builds I had lying around from the last regression-window search, I pulled up the 2005-05-01-08 build (before global history in the Go menu).

In that build, the iframes show up in the manager/about:history!  So it seems like 1) something fixed this so the iframes didn't appear in about:history but 2) that fix didn't get applied to the parsing for the Go menu when global history moved there.  I'll check some more builds that do have global in Go and see if I can find anything else useful.

Note I typically--at least of late--have to go to click on the "page 2" link at the bottom of macosxhints.com to get the iframes to show up; the home page doesn't seem to throw them, or there's something in the ad rotation that makes the second page more likely to get the bad iframes?

Comment 12

[Simon Fraser [no longer active]]

(In reply to comment #11)
> This is interesting.  
> 
> Currently, these iframes only appear in the Go menu global history, not the
> session history on the Back button nor the global history in
> about:history/Bookmarks Mgr.  That seems odd, as I'd think that Go menu global
> history = about:history global history.

Me too; they use the data data source.

Comment 13

[Simon Fraser [no longer active]]

s/data data/same data

Can someone code up a testcase?

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Never mind that; I see them in both global history places now in tonight's branch nightly--but I'm positive I didn't before.  Odd.

One interesting note, though.  In Mike's short-lived (4-21 and 4-22 only) v1 of global history in the Go menu, the iframes didn't appear in the Go menu (but did appear in about:history).  I assume that impl didn't use the same data store?  

v2 (Simon's impl) seems to have the iframes showing up in both Go and about:history as soon as it first landed (5-14).

Also, the iframes apparently disappear from history when you quit Camino.

I am sooo confused....

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

FWIW, doesn't happen in SeaMonkey (branch nightly from the 27th), either.  Is bug 148794 embedding only (it doesn't seem like it), or has everyone else just figured out a way to patch around this problem?

Back in Camino, I just had the iframes appear in Go but not in History again on page 2 of macosxhints; going to the next page seemed to make them appear, though--as the topmost items, above page 3, so they were actually loaded on page 3 and not just a late carry-over from page 2/leaving page 2.

Comment 16

[Mike Pinkerton (not reading bugmail)]

i don't know what you guys are seeing. i'm using |2005101814| and i do NOT see iframes show up in the go menu.

using a current branch nightly, i do.

Comment 17

[Samuel Sidler (old account; do not CC)]

I do using 2005101814 as well as the more recent 2005102314. Like Smokey says, they're not in about:history though.

Comment 18

[Simon Fraser [no longer active]]

Attachment: Testcase

Comment 19

[Simon Fraser [no longer active]]

iframe site shows up both in the menu, and in the about:history for me.

Comment 20

[Simon Fraser [no longer active]]

The call that adds the global history entry comes right out of nsDocShell::OnNewURI(). We should not block for this; there's no way they'll let a fix in.

Did someone check to see if this happens in FF?

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Yeah, I see them in 2005101814, too.  On the main macosxhints page, they only showed up in about:history, but on the second page, the iframes were in about:history and in the Go menu.

You should see them with Gmail, too.

Given how random and inconsistent this behavior is (sometimes in Go, sometimes not; sometimes in about:history, sometimes not) and the fact this has been occurring since we added global history to the Go menu, and given the fact 1.0a1 is almost two months old, I don't think we should hold Beta for this....

Yes, I've checked and Fx (and Sm) don't have these iframes showing up in their Go menus or History.  It's only in Camino.

Comment 22

[Simon Fraser [no longer active]]

So URI loads for frames get a "hidden" flag set on them in the history. We need to ignore these when building the cocoa data sources.

Comment 23

[Simon Fraser [no longer active]]

Attachment: Patch

This patch avoids making HistorySiteItems for hidden items.

Comment 24

[Simon Fraser [no longer active]]

Fixed, trunk and branch.

Comment 25

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Um, I just saw this in the Go menu with page 2 of macosxhints and in history proper with page 3, Camino 2005111004 (v1.0b1+) :-(

Same 3 iframes as in comment 7.  What's up with that?

Comment 26

[Simon Fraser [no longer active]]

I think what you are seeing is older history entries whose "last visit" date was jump bumped up to "today"; they may have lost their "hidden" flags somehow.

The real test to see if this bug is fixed is to clear your history, then go back and look for iframe urls showing up.

Comment 27

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Unfortunately, that's not the case.  I moved aside by old history.dat, launched Camino, and visited macosxhints.  The iframes on page 2 were a little slow loading, so I actually watched them appear in about:history, and when I checked the Go menu, they were there as well.

(They don't seem to persist in history, however, even within a session.  macosxhints was one of the last things I visited in Camino this morning; I didn't quit, and now when I came back to Camino and searched my old history before moving it aside, the iframes didn't show up.)

Comment 28

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Some more data: the iframes disappear from about:history when you close it, and if you don't have about:history open when you're loading a page that has these iframes, they'll never show up in about:history at all.  

That explains why some of us were seeing the iframes in history (and some of the time) and others were not, although not why the iframes are still showing up when about:history is open.

That still doesn't explain why they're showing up in the Go menu, though (at least not to me), since even if about:history is closed, the iframes are still in the Go menu (and they stick around).  Is it possible the menu-generating code is ignoring the hidden flag?

Comment 29

[Simon Fraser [no longer active]]

Ah, I get it. There's one other place I have to filter "hidden" loads.

Comment 30

[Simon Fraser [no longer active]]

Attachment: Patch to filter out newly added "hidden" items

This should do the trick.

Comment 31

[Simon Fraser [no longer active]]

Really fixed now.

Comment 32

[Simon Fraser [no longer active]]

Attachment: Secure iframe testcase (just testing)