415319 - Make the feedhandlers appear like part of Camino (no alerts on launching, auto-appear in the menu)

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

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

Mark mentioned that we want to make the feedhandlers seem like part of Camino and not have them triggering first-run/quarantine alerts all over the place.  

The proposed solution was to "exec" them at some point after install or Camino's first launch.

Comment 1

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

This is actually more important than just alerts; in some (all?) cases where the apps weren't constructed on the machine running them, LS won't show them in our feed menu until they've been launched (open path/to/Camino.app/Contents/Resources/$FeedHandler.app/Contents/MacOS/applet) and the General prefPane switched away from and back again.

This effectively prevents feedhandlers from being used in those cases :(

Comment 2

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

Plussing and targeting at 1.6 based on the last comment.

Comment 3

[Stuart Morgan]

I'll post a patch tomorrow that will solve the more serious comment 1 without actually fixing the original bug (by registering the apps with LS, rather than launching them). The problem with actually launching them is that they steal focus for just a second, which creates an odd flashing in the frontmost window.

That seems undesirable in the normal use case as well; is there any way to change the handlers to prevent them from doing that?

My thought is that the partial solution is enough to make them testable in b3, and we can see about improving things after that.

Comment 4

[Stuart Morgan]

Attachment: register on launch

I'll test this later when I have a machine that hasn't been doing builds, but if anyone else has a machine where the handlers have never been registered and wants to give this a try please do.

Comment 5

[Stuart Morgan]

Comment on attachment 310471 [details] [diff] [review]
register on launch

This patch works on 10.5, and probably 10.4, but apparently not on 10.3 (probably due to our old friend, the LS bug that treats apps that haven't been opened as non-existent). I'd say that we should land this as a partial fix, since the approach is basically what we would want for launching the apps once we can do that without focus weirdness (just replacing the LS call with an open).

Comment 6

[Stuart Morgan]

Attachment: full fix

Smokey found LSBackgroundOnly; between that and setting an explicit background launch flag using an LS call directly instead of NSWorkspace, no more flashing. So this should fix 10.3, and gets rid of the first-launch warning.

Comment 7

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

Comment on attachment 310633 [details] [diff] [review]
full fix

r=ardissone; great job!

Comment 8

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

Er, I didn't test the "no quarantine alerts" on 10.5 since I don't have a 10.5 machine that's not being used for building, but 10.3 definitely gets me a launch free of any first-launch warnings.

Also, this should block 1.6b3 now that Stuart's worked his magic.

Comment 9

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

(In reply to comment #8)
> Er, I didn't test the "no quarantine alerts" on 10.5 since I don't have a 10.5
> machine that's not being used for building

I did download my build in another account, note that the account showed no feedhandlers, and ran the build and handlers without getting any Quarantine notices, so I suspect we are indeed good there, too.

Comment 10

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

This is technically going to be a tiny bit of late-l10n, since feedhandlers are moving from Resources to Resources/FeedHandlers.  

In a best-case scenario, the tools will notice the strings have moved and update automatically; in the worst case, there's just a few strings that will have to be copy/pasted around.

Comment 11

[Stuart Morgan]

It hadn't occurred to me that there would be any impact; if that's going to be a serious problem I can move them back for 1.6 and change the code to pick them out of everything else in the folder.

Comment 12

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

It hadn't occurred to me before today, either.  Marcello, Markus, what do you think is best?

Comment 13

[Marcello Testi]

I think at least half of existing teams should be already down at localizing 1.6 at this point. It's also true that these guys are likely to be quick in picking up small fixes to do, if properly and timely instructed. So, my answer, is: yes, let's keep the changes and quickly land them, pointing the list and website users to a workable build. I will pre-announce this on the list.

Comment 14

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 310633 [details] [diff] [review]
full fix

sr=pink

we seem to have so much going on at startup on separate threads. at what point does it begin to seriously impact startup time?

Comment 15

[Stuart Morgan]

This may impact startup time, but it would only be once per new version. We'll see it in Ts if it's a problem though, and if it is we could delay it for a little while since it's unlikely to matter immediately at startup.

Comment 16

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH.

Comment 17

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

For the record, cb-x1 branch and trunk both went up 3-4ms (1% on branch), but boxset went up from around 1420ms to about 1950ms, which is a whopping 37% regression.

Comment 18

[Stuart Morgan]

Filed bug 424885 for comment 15/17.

Comment 19

[Mark Mentovai]

Comment on attachment 310633 [details] [diff] [review]
full fix

-	<key>LSUIElement</key>
+	<key>LSBackgroundOnly</key>
 	<string>1</string>

Nit: keep the plist alphabetized.

<string>1</string> looks odd, does <true/> work?

Comment 20

[Stuart Morgan]

(In reply to comment #19)
> Nit: keep the plist alphabetized.

Fix landed.

> <string>1</string> looks odd, does <true/> work?

IIRC, at least LSUIElement used to be picky; either would work in 10.5, but I can't test back to 10.3 easily. If we want to verify on 10.3, we can change that too.

Comment 21

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

It's a little bizarre: the docs (last revised 2007-08-23) definitely call for 'String' http://developer.apple.com/documentation/Carbon/Conceptual/LaunchServicesConcepts/LSCConcepts/chapter_2_section_4.html#//apple_ref/doc/uid/TP30000999-CH202-DontLinkElementID_3
and use it over and over in preference to anything else, but they also mention that on 10.2 or later you can use 'Boolean' or 'Number'.

I can test-to-verify later, if we want.