468881 - Save Frame As… option appears when not necessary in File Menu

Status: VERIFIED FIXED

(Firefox :: Menus, defect)

Description

[Toni Hermoso Pulido]

I've noticed 'Save Frame As…' option appears when not necessary in File menu.
For instance in webpages I do not see any frame or iframe in their source code:
http://support.mozilla.com/

And even in pages where we do have frames, is there any use in having this option in main File menu? I consider it's only useful when right-clicking within a given frame space.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Good examples are error pages on mozilla.com like the one you get when opening the following URL: http://en-us.www.mozilla.com/en-US/firefox/9.5/whatsnew/

It is still a problem on all platforms with the Firefox 3.5 RC1 build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090612 Firefox/3.5 ID:20090612061417

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Hm, I can't reproduce this wit hMozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a2pre) Gecko/20090817 Firefox/3.6a2pre

RESO WFM?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Still visible on the given url from comment 1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090908 Minefield/3.7a1pre ID:20090908030622.

Also with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090907 Namoroka/3.6a2pre (.NET CLR 3.5.30729) ID:20090907045039

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Hm, interestingly, it only shows up once the page load is complete. As the page is loading, it doesn't.

The variable is flipped based on this code:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#160

Anyone have an idea what could cause that check to go awry?

(Also, I'd support taking this out of the File menu entirely, yes)

Comment 5

[Boris Zbarsky [:bzbarsky]]

That check isn't going awry.  The page has an iframe on it:

  <iframe id="_yuiResizeMonitor" style="position: absolute; visibility: visible; width: 10em; height: 10em; top: -134px; left: -134px; border-width: 0pt;"/>

Now that iframe isn't present in the _source_, but presumably one of the mess of YUI scripts on that page adds it.  And presumably it does it onload or on DOMContentLoaded or something, which would explain the behavior Mike is seeing.

If really desired I can track down exactly who's adding it and when, of course.

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch to remove menuitem

(In reply to comment #4)
> (Also, I'd support taking this out of the File menu entirely, yes)

This patch does just that. Note that the functionality is still available via the context menu (This Frame -> Save Frame As...). 

I've also removed the supporting function saveFrameDocument() from toolkit/content/contentAreaUtils.js, as its no longer used anywhere.

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/2910f583aa98

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on trunk with builds on Windows and OS X like Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091003 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091003042919

Blair, is this patch ready to ask for approval1.9.2?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 400680 [details] [diff] [review]
Patch to remove menuitem

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Litmus test has been updated:
https://litmus.mozilla.org/show_test.cgi?id=10036