188604 - Open viewsource in a tab or window should reflect users prefs

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, enhancement, P3)

Description

[J Luh]

When the user selects View Source, perhaps the source should open in a new
window rather than a new tab if the user has selected "Loading a page requested
by another application: Opens a new window" in preferences.

Comment 1

[saari (gone)]

->sfraser

Comment 2

[Jasper]

This would be a bit of a strange solution since Chimera (the user) makes the
request and not another application?

But I do agree we should have a preference option where the user could select if
he/she wants to open the source in a tab attached to the html window or a new
window.

Comment 3

[J Luh]

Re: Comment 2, true enough--the preference refers to pages requested by other
applications. I guess what I was suggesting was a universal preference that
would let users indicate a single time whether they prefer tabs or windows.
Letting users select tabs or windows for different things (view source, windows
requested by other applications, etc) would provide the maximum flexibility, but
it might unnecessarily clutter the preference pane.

Comment 4

[Chris Casciano]

*** Bug 205280 has been marked as a duplicate of this bug. ***

Comment 5

[Wes Felter]

If the user has both tab prefs set to "Opens a new window", it's pretty clear
that they don't want tabs to be created, so Camino should open a new window. But
a "never ever create tabs" pref would be fine, too.

Comment 6

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

This should open in a new window unless the user has every option for new
windows turned off. In addition, this bug should be dependent on the
single-window browsing bug (which I can't seem to locate atm). If that is
"checked" (after implemented), then view source opens in new window. Otherwise,
new tab.

Comment 7

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

View Source is the only thing I ever want opening in a new window, so I can look
at the page and the source at the same time.  But then I'm odd.

From a logical point-of-view (and a let's not clutter Camino with tons of prefs
no one cares about) it should probably follow one of the existing prefs.

SWM is bug 274143.

Comment 8

[Jon Hicks]

I agree with Smokey, while I want everything to be opened in a new tab, I prefer
view source to be in a new window, so that I can compare the source with the
page. As far as I know, Camino is the only browser to open view source in a tab,
rather than a new window. Should'nt a new window be the default behaviour? 

Comment 9

[Jasper]

I agree, it probably should. Come on guys most web devs agree on this, come on
fix this. It can't be that hard.

Comment 10

[Wevah (Nate Weaver)]

I actually like View Source opening in a new tab, FWIW.

Comment 11

[Simon Fraser [no longer active]]

If we open a new browser window, it will be a fully-fledged browser window with
all the toolbars etc. That's a little different than Firefoxes simple source window.

Comment 12

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

To me even a full-fledged window is better than a tab; at the very least, it
saves me a "move to new window" cm (if the view-source tab is even among the
visible ones which gets that cm)....

Comment 13

[Chris Lawson (gone)]

Would it be horribly wrong to make a new pref for this?

Something like browser.viewsource.target; 0 could be tab and 1 could be window. The UI could go in the Tabs prefs pane, or we could leave it hidden.

The reason I suggest a new pref is because I (also) would prefer View Source open in a new window, but for EVERYTHING else, I want new tabs (and have therefore disabled all new window options, which would make me fall victim to Sam's suggestion in comment 6). I sort of doubt I'm alone in this preference, and supporting another pref would be pretty trivial here. So trivial, in fact, that I already have a patch.

cl

Comment 14

[Simon Fraser [no longer active]]

I think a new pref might be appropriate here (probably not exposed though).

Comment 15

[Chris Lawson (gone)]

Oh, also, I don't think we should be loading in the background, regardless of the loadInBackground pref. Neither Safari nor Firefox loads source in the background, and I think it's a safe assumption that people choosing "View Source" want to see it right now, without having to do anything extra to bring the source window to the front.

cl

Comment 16

[Chris Lawson (gone)]

Attachment: loads view source according to new pref, always in foreground

This creates a Boolean pref (browser.viewsource.usetabs) that should be set to TRUE if the user wants View Source to use a tab. Otherwise, it will use a window. Either way, the HTML source loads in the foreground.

cl

Comment 17

[Chris Lawson (gone)]

Attachment: respects shift key, better pref name

Comment 18

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

*** Bug 324837 has been marked as a duplicate of this bug. ***

Comment 19

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 209251 [details] [diff] [review]
respects shift key, better pref name

sr=pink

i guess we're ok with not respecting the loadinBG pref, but it seems like something we'll have to explain over and over as new bugs come in....maybe not.

Comment 20

[Chris Lawson (gone)]

Attachment: suppresses toolbars when creating view-source window

This has been totally re-worked to use chrome flags to suppress toolbars on the view-source window. View-source-in-tab behaviour remains the same as before. There's been some associated logic cleanup in |loadSourceOfURL|.

In case anyone is wondering why I modified it to take an external loadInBackground parameter, the GetCurrentEventKeyModifiers() works better the higher up you call it, so I'm looking for it when the two IBActions that call |loadSourceOfURL| are triggered instead.

cl

Comment 21

[Torben]

Re attachment 210333 [details] [diff] [review]

+  BOOL shouldUseTab = [[PreferenceManager sharedInstance]
   getBooleanPref:"camino.viewsource_in_tab" withSuccess:NULL];

Do we actually want to change the default behaviour to open view source in a window? IMHO it would be better to have a pref "camino.viewsource_in_window".

Comment 22

[Chris Lawson (gone)]

(In reply to comment #21)
> Re attachment 210333 [details] [diff] [review] [edit]
> 
> +  BOOL shouldUseTab = [[PreferenceManager sharedInstance]
>    getBooleanPref:"camino.viewsource_in_tab" withSuccess:NULL];
> 
> Do we actually want to change the default behaviour to open view source in a
> window? IMHO it would be better to have a pref "camino.viewsource_in_window".

General consensus on IRC seemed to be that while we're fixing this, we should bring Camino's default behaviour in line with that of every other browser in the universe.

cl

Comment 23

[Mike Pinkerton (not reading bugmail)]

i dunno. people aren't complaining that it opens in a tab. A select few are complaining that they can't make it open in a window if they want to. Is that a reason to change the default?

Comment 24

[Chris Lawson (gone)]

In response to Torben and Mike's comments...

We ship Camino with every single option for tabs turned off. The tab bar is off, cmd-click opens links in a new window (rather than a tab), links from external apps open in a new window (rather than a tab), etc.

So why does (or should) view-source open in a tab, in stark contrast to every other pref default? That doesn't make much sense to me.

cl

Comment 25

[Jasper]

I agree with Chris, viewsource is somehow a special case for always opening it in tabs, while in reality those who will use it most probably want to view it in a separate window so they can compare it with the actual page.

Comment 26

[Simon Fraser [no longer active]]

Comment on attachment 210333 [details] [diff] [review]
suppresses toolbars when creating view-source window

>Index: BrowserWindowController.mm
>===================================================================


> 
>+static NSString* const ViewSourceProtocolString = @"view-source:";

kViewSourceProtocolString please.

Otherwise it looks OK.

Comment 27

[Chris Lawson (gone)]

Attachment: fixes Simon's comment

Transferred r+ flag from old patch. Sorry 'bout forgetting that k :)

Comment 28

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 213720 [details] [diff] [review]
fixes Simon's comment

sr=pink, though the patch itself has a typo and doesn't compile. i'll fix, merge with the tip, and land.

Comment 29

[Mike Pinkerton (not reading bugmail)]

fixed.

Comment 30

[Torben]

> In case anyone is wondering why I modified it to take an external
> loadInBackground parameter, the GetCurrentEventKeyModifiers() works better the
> higher up you call it, so I'm looking for it when the two IBActions that call
> |loadSourceOfURL| are triggered instead.

So Cmd-Opt-Shift-v should open the source tab/window in the background, right? For me it just beeps without opening anything :-( Cmd-Opt-v works fine(ie opens the source tab/window in the foreground). OS X 10.3.9, 1.8-branch build.

Comment 31

[Chris Lawson (gone)]

(In reply to comment #30)
> So Cmd-Opt-Shift-v should open the source tab/window in the background, right?
> For me it just beeps without opening anything :-( Cmd-Opt-v works fine(ie opens
> the source tab/window in the foreground). OS X 10.3.9, 1.8-branch build.

That might be a 10.3-specific issue; this works as expected in my current trunk nightly (although it would probably give me carpal tunnel syndrome faster than just about any other keyboard shortcut I can think of if I used it regularly).

If you're still seeing broken behaviour here, please file a new bug for it.