If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

When Camino is hidden, invoking via Service does not respect external app pref



Camino Graveyard
Tabbed Browsing
12 years ago
12 years ago


(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Simon Fraser)


({fixed1.8.0.4, fixed1.8.1, regression})



(2 attachments, 2 obsolete attachments)

1. Set "Link from other application" to "Opens in new tab in the frontmost window" 
2. Hide Camino
3. Go find a URL somewhere, e.g. in a TextEdit document
4. Select the URL and cmd-shift-u

Expected: Camino unhides and opens said url in new tab in my current Camino window
Actual: Camino unhides and opens said url in a new window

I'm pretty sure this is a regression, but I don't have a window yet.  I noticed it in 2005122504 (v1.0b1+).  It works correctly when Camino is not hidden.

Comment 1

12 years ago
What's the difference between a call from an external app and a call from a Service? (Other than the result of this bug, I mean.)


Comment 2

12 years ago
The services call comes in through MainController's - (void)openURL:(NSPasteboard *) pboard userData:(NSString *) userData error:(NSString **) error;

Comment 3

12 years ago
Oh, my -getFrontmostBrowserWindow changes probably broke this.
Assignee: mikepinkerton → sfraser_bugs
It works in 2005121804 and is broken in 2005121904, so (assuming no wonkiness from the 1.8/1.8.0 switch-over) it looks like your first (12-18) -getFrontmostBrowserWindow change broke it and the follow-up on 12-19 didn't fix it.
Target Milestone: --- → Camino1.1

Comment 5

12 years ago
I think this is the same problem I have while using NetNewsWire (tested with both NNW 2.01 release and 2.1b17 latest beta at time of writing):

* Hide Camino.
* bring NNW to the top
* click on link in NNW (any link)

Result: Camino is unhidden, but the link opens in a new window instead of a new tab, as expected from my prefs.

Note: Camino stays (correctly) at the bottom of the stack, per my prefs in NNW (Open links in the background).

Comment 6

12 years ago
I can repro comment 5 in the 1.0 release.

Hmm, I actually can't repro with standard external app, and comment 1 sounded like Chris couldn't, either, but (to me) it's plausible they're still related ;)

As I think this is the only known regression we shipped in 1.0, nominating for 1.0.1, too.

Related question about another bug needing some QA: do either of you (philippe, Chris) see bug 326091, too?
Flags: camino1.0.1?

Comment 8

12 years ago
> Related question about another bug needing some QA: do either of you (philippe,
> Chris) see bug 326091, too?

I'm neither philippe nor Chris, but I can repro Comment #5, and I don't see bug 326091.

Comment 9

12 years ago
(In reply to comment #7)

> Related question about another bug needing some QA: do either of you (philippe,
> Chris) see bug 326091, too?

Just tested that one, and no, no problems on my side (10.4.5, latest trunk build).

Blocks: 333974

Comment 10

12 years ago
I don't see these coming in via the service, I see them coming in as GURLs.

Comment 11

12 years ago
Comment 10 applies to bug 333974.  From now on, this bug is only about services as far as I'm concerned, as Smokey initially intended.

Comment 12

12 years ago
Created attachment 218939 [details] [diff] [review]
Unhide app before loading

This is one way to go.  The app unhides in both cases anyway, but not soon enough.
Attachment #218939 - Flags: review?(sfraser_bugs)

Comment 13

12 years ago
In GetURLCommand.mm:

+  [NSApp unhide:self];

was supposed to be

  [NSApp unhideWithoutActivation];

Comment 14

12 years ago
Created attachment 218996 [details] [diff] [review]
Unhide app before loading (fixed)
Attachment #218939 - Attachment is obsolete: true
Attachment #218939 - Flags: review?(sfraser_bugs)

Comment 15

12 years ago
Created attachment 218997 [details] [diff] [review]
Unhide app before loading (fixed)

Whoops, wrong patch.
Attachment #218996 - Attachment is obsolete: true

Comment 16

12 years ago
Created attachment 218998 [details] [diff] [review]
Check for hidden app when finding [frontmost] browser window[s]

There was a "flicker" with the other patch because the app would unhide before a new tab would open.  The old tab's contents would be visible for a small (and ugly) amount of time.
Attachment #218998 - Flags: review?(sfraser_bugs)

Comment 17

12 years ago
Comment on attachment 218998 [details] [diff] [review]
Check for hidden app when finding [frontmost] browser window[s]

Stictly speaking this change could allow you to get a partially constructed, or being-torn-down window, but that's pretty unlikely.
Attachment #218998 - Flags: review?(sfraser_bugs) → review+
mento and i talked about this, we're not going to do this for 1.0.1 as we have nothing landed on the trunk and it's a little iffy. We'll try for 1.0.2 if there is such a beast.
Flags: camino1.0.1? → camino1.0.1-
Since this is our only known regression in 1.0, it would be nice to get it in 1.0.2. So I'm nominating for that release so we can ponder it again later.
Flags: camino1.0.2?

Comment 20

12 years ago
I think this broke pre-1.0.
Yes, this is one of 2 known regressions we shipped in 1.0 (it broke shortly before 1.0, was filed, and then was not fixed before 1.0); the other was bug 318001 (which we didn't actually figure out the cause of before 1.0).

At any rate, we do need to get testing on the trunk, see if it fixes bug 333974 too, doesn't regress anything else, etc., before landing on the stable branch--no argument here ;)
QA Contact: tabbed.browsing

Comment 22

12 years ago
This does fix the other bug too.

Comment 23

12 years ago
Checked in on trunk, 1_8, and 1_8_0 branches.  It seems pretty unlikely to be creating or tearing down windows while the app is hidden, but if there's a better approach, I'm game.
Last Resolved: 12 years ago
Keywords: fixed1.8.0.3, fixed1.8.1
Resolution: --- → FIXED
Keywords: fixed1.8.0.3 → fixed1.8.0.4
Er, plussing since this landed for
Flags: camino1.0.2? → camino1.0.2+
You need to log in before you can comment on or make changes to this bug.