175863 - add Reload All Tabs to tab context menu

Status: VERIFIED FIXED

(Camino Graveyard :: Tabbed Browsing, enhancement)

Description

[sairuh (rarely reading bugmail)]

rfe: add Reload All Tabs to tab context menu. mozilla also has this in its tab
context menu.

Comment 1

[Pete Doane]

this sounds like a good idea.  it should also be put in the view menu with a
shortcut of command-shift-r.

Comment 2

[Simon Males]

I guess the shortcut part of this bug would be bug 216083 

Comment 3

[Art Casey]

This is a great idea and as previously stated, it's in the Mozilla browser.
While I was using Mozilla, I found the "reload all tabs" feature very useful.

Comment 4

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

Command-shift-R is going to be taken by a reload from server (not from cache),
so it won't be used for reload all tabs. I disagree that it should be added to
the view menu. Then we have to deal with the issue of whether we should rename
the other option "Reload Tab" or have it's name change when more than one tab is
open and have the other option disabled by default. Lots of issues there.

But, reload all tabs is a good idea. Seems fairly easy, but targeting for 1.2.

Comment 5

[Matt Smith]

Oh, please make this more urgent!  Throughout the day I'll have somewhere
between 3 and 5 discussion groups open in one tabbed window.  Reload all tabs
would make that so much more convenient!

M@

Comment 6

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

(In reply to comment #4)
> Command-shift-R is going to be taken by a reload from server (not from cache),
> so it won't be used for reload all tabs. 

Opt is the standard additional modifier for "all", so Cmd-Opt-R should be
good--assuming it's not used elsewhere or stolen globally by the OS.

Comment 7

[John Bailey]

A contextual menu when clicked in the empty space of the tab bar would be nice too.

Comment 8

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

(In reply to comment #7)
> A contextual menu when clicked in the empty space of the tab bar would be nice
too.

That's bug 291901 comment 6; this bug is about adding an option to the exiting
tab context menu.

Comment 9

[Chris Lawson (gone)]

Attachment: Patch for implementation of reloadAllTabs method

Here's the code.

Need an opinion on the NIB files -- do we want to put a Reload All Tabs option
in the View menu, or just in the tab context menu?

cl

Comment 10

[Simon Fraser [no longer active]]

Comment on attachment 195836 [details] [diff] [review]
Patch for implementation of reloadAllTabs method

>Index: src/browser/BrowserWindowController.mm
>===================================================================
>RCS file: /cvsroot/mozilla/camino/src/browser/BrowserWindowController.mm,v
>retrieving revision 1.208
>diff -u -r1.208 BrowserWindowController.mm
>--- src/browser/BrowserWindowController.mm      12 Sep 2005 20:09:00 -0000      1.208
>+++ src/browser/BrowserWindowController.mm      13 Sep 2005 04:20:57 -0000
>@@ -2591,6 +2591,22 @@
>   }
> }
> 
>+- (IBAction)reloadAllTabs:(id)aSender
>+{
>+  unsigned int reloadFlags = NSLoadFlagsNone;
>+  NSEnumerator* tabsEnum = [[mTabBrowser tabViewItems] objectEnumerator];
>+  id curTabItem;

C++ style declare on usage, please.

>+
>+  if (([[NSApp currentEvent] modifierFlags] & NSShiftKeyMask) != 0)
>+    reloadFlags = NSLoadFlagsBypassCacheAndProxy;
>+
>+  while ((curTabItem = [tabsEnum nextObject]))
>+  {
>+    if ([curTabItem isKindOfClass:[BrowserTabViewItem class]])
>+      [[[curTabItem view] getBrowserView] reload: reloadFlags];
>+  }
>+}

Otherwise looks good. And I don't think we want this on the main menus, just
the context menu.

Comment 11

[Wevah (Nate Weaver)]

(In reply to comment #10)

> >+      [[[curTabItem view] getBrowserView] reload: reloadFlags];

No spaces between argument names and arguments, too, IIRC (e.g., this should be
[[[curTabItem view] getBrowserView] reload:reloadFlags];).

Comment 12

[Simon Fraser [no longer active]]

> >+- (IBAction)reloadAllTabs:(id)aSender
> >+{
> >+  unsigned int reloadFlags = NSLoadFlagsNone;
> >+  NSEnumerator* tabsEnum = [[mTabBrowser tabViewItems] objectEnumerator];
> >+  id curTabItem;
> 
> C++ style declare on usage, please.

By this I mean do this:

  unsigned int reloadFlags = NSLoadFlagsNone;
  if (([[NSApp currentEvent] modifierFlags] & NSShiftKeyMask) != 0)
    reloadFlags = NSLoadFlagsBypassCacheAndProxy;

  NSEnumerator* tabsEnum = [[mTabBrowser tabViewItems] objectEnumerator];
  id curTabItem;
  while ((curTabItem = [tabsEnum nextObject]))
  {
    ...

Comment 13

[Chris Lawson (gone)]

Attachment: Updated per Nate and Simon's suggestions

Fixed. Will upload the patched NIB file in a minute.

Nate, the space-before-argument problem is widespread in the file. I've fixed
it here, but there are several (like 20 or 30) instances of it elsewhere in
BrowserWindowController.mm. Someone should double-check that when this gets
checked in.

cl

Comment 14

[Chris Lawson (gone)]

Attachment: Zipped-up BrowserWindow.nib file with Reload All Tabs item in it

Here's the NIB file.

cl

Comment 15

[Wevah (Nate Weaver)]

(In reply to comment #14)
> Created an attachment (id=195922) [edit]
> Zipped-up BrowserWindow.nib file with Reload All Tabs item in it
> 
> Here's the NIB file.
> 
> cl

Yeah, I know. Too many cooks, and all that. ;) I think it was talked about a bit
in IRC ages ago, though.

Comment 16

[Chris Lawson (gone)]

OK, for the record, I've been using this patch for about a week or so now, and I
*really* want a keyboard shortcut for it. Having to resort to the mouse to
reload all tabs in the current window is driving me nucking futs.

Are there any particular objections to adding a menu item in the View menu,
immediately under "Reload Page"?

I'm just not sure what to call it. I can see arguments both for and against
"Reload All Tabs," but that's probably my current favourite.

cl

Comment 17

[Chris Lawson (gone)]

Attachment: New version, fixing a bunch of style errors in the file

Comment 18

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

(In reply to comment #16)
> Are there any particular objections to adding a menu item in the View menu,
> immediately under "Reload Page"?

Ideally we'd do this with Cmd-Opt-R and have the "Reload Page" menu item change
name and shortcut when Opt is held down, but I think we have to drop 10.2
support first (Wevah has a bug about this somewhere).
 
> I'm just not sure what to call it. I can see arguments both for and against
> "Reload All Tabs," but that's probably my current favourite.

"Reload Current Tabs" has similar issues, I think.

Needless to say, I do think it is useful enough to warrant a keystroke....

(If that gets approved and bug 290520 hasn't been checked in yet, can you put
those changes in your nib, too, so there's one less checkin and a better chance
of getting it all correct?)

Comment 19

[Wevah (Nate Weaver)]

"Reload All Tabs in Window" says what it does perfectly, though it might be a
bit too wordy.

Comment 20

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

-> cl

Comment 21

[Jasper]

Pong, could we get this before christmas :D I'd really like to have this soon.

Comment 22

[Chris Lawson (gone)]

Attachment: Updated for latest code revisions as of today

Updated to fix CVS merge breakage, asking for r from pink.

cl

Comment 23

[Chris Lawson (gone)]

Comment on attachment 196743 [details] [diff] [review]
New version, fixing a bunch of style errors in the file

clearing review flag on obsolete attachment

Comment 24

[Mike Pinkerton (not reading bugmail)]

please give me a diff -w to ignore all the whitespace changes. too much noise
here to review.

Comment 25

[Chris Lawson (gone)]

Attachment: Whitespace changes ignored

Woops. Meant to do this a couple days ago per Simon's note, but forgot. Sorry
'bout that. Here's the new one.

cl

Comment 26

[Chris Lawson (gone)]

Also, per comment 18, I agree we should wait on adding a menu item/shortcut in
the View menu until we drop 10.2 support and can do it properly. Should I open
up a new bug for that once this gets checked in?

cl

Comment 27

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 199037 [details] [diff] [review]
Whitespace changes ignored

looks good to me

r=pink

Comment 28

[Jasper]

Bump

Comment 29

[Simon Fraser [no longer active]]

Checked into trunk and branch.

Comment 30

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

I filed bug 312904 on comment 18/comment 26.