272 bytes, text/html
1.12 KB, patch
|Details | Diff | Splinter Review|
726 bytes, patch
|Details | Diff | Splinter Review|
283 bytes, text/html
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.
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)?
See also bug 237717.
since bug 237717 was fixed, has this also been fixed?
(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. ;)
Yes confirming, this bug is still present in the 2005-11-01 branch build.
This has to be core. Is it bug 148794?
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
on what day did this regress? this is a serious beta blocker and must have been a recent core change to cause it.
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?
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 :(
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?
(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.
s/data data/same data Can someone code up a testcase?
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....
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.
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.
I do using 2005101814 as well as the more recent 2005102314. Like Smokey says, they're not in about:history though.
iframe site shows up both in the menu, and in the about:history for me.
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?
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.
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.
Created attachment 201909 [details] [diff] [review] Patch This patch avoids making HistorySiteItems for hidden items.
Fixed, trunk and branch.
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?
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.
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.)
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?
Ah, I get it. There's one other place I have to filter "hidden" loads.
Created attachment 202778 [details] [diff] [review] Patch to filter out newly added "hidden" items This should do the trick.
Really fixed now.