512628 - Applescript: visit count property of bookmarks broken in 2.x

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Lisa Thompson]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en; rv:1.9.0.15pre) Gecko/2009082501 Camino/2.1a1pre (like Firefox/3.0.15pre)
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en; rv:1.9.0.15pre) Gecko/2009082501 Camino/2.1a1pre (like Firefox/3.0.15pre)

Accessing the visit count property of a bookmark generates an "AppleEvent handler failed" error message. Happens in 2.x only; works fine in 1.6.8.

STR
1. Open script editor and paste this script into a new window:
tell application "Camino"
  set aBookmark to bookmark "BBC News" of bookmark folder "News" of bookmark bar collection
  return visit count of aBookmark
end tell
2. Launch Camino 2.x with a fresh profile.
2a. (optional) Choose the BBC News bookmark a few times to rack up the visit count
3. Switch back to script editor and click "Run".

AR
Errors on second line of script, "AppleEvent handler failed".

ER
The number of visit counts for the BBC bookmark should appear in the Result pane.

I tried some old 2.0a1pre nightlies to see when this might have broken, but I'm not sure it ever worked, or perhaps it was broken by an OS update. I'm on 10.5.8, but I think I noticed this as early as 10.5.7.

In 2.0a1pre of 01-Sep-07, the script errors on the first line with "can't get bookmark bar collection", so I guess the feature wasn't fully implemented yet? In 2.0a1pre of 01-Oct-07, it errors on the second line with "AppleEvent handler failed", and that continued as I tested nightlies of every 6 months or so and milestones 2.0a1 to 2.0b3. All tests with clean profile.



Reproducible: Always

Comment 1

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

AppleScript bookmark support landed on 2007-09-02 (both trunk and branch), so 2007-09-03 nightly is what you'd want to check to be sure, but it sounds like it never worked on trunk.  That's odd.

I don't know if the sdef compiling back to .scriptSuite and .scriptTerminology somehow paves over this in 1.6.x (pulling the .sdef from 2.x and the .script* from 1.6.x and swapping them might tell); otherwise, it's got to be some bizarre change to bookmarks code, but that shouldn't have diverged until after 1.6 was released.

(:sigh: Bug 453202 :sigh:)

Comment 2

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

(If you play with swapping, you'll also need to swap in or out the Info.plist key for the .sdef: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/Info-Camino.plist.in&rev=1.14&mark=223-224#223)

Comment 3

[Lisa Thompson]

re comment 1: AppleScript bookmark support landed on 2007-09-02 (both trunk and branch), so 2007-09-03 nightly is what you'd want to check to be sure, but it sounds like it never worked on trunk.

I get the "AppleEvent handler failed" message with 2.0a1pre from 03-Sep-07.

Comment 4

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

So, its the .sdef :/  I swapped out the .sdef and in the .script* and .rsrc, and 2.x works fine. I subbed the .sdef in in 1.6 and it faileds.

.sdef:

<property name="visit count" code="pvct" description="The number of times the bookmark has been visited." type="number" access="r">
	<cocoa key="numberOfVisits"/>
</property>

vs.

.scriptSuite:

<key>numberOfVisits</key>
<dict>
	<key>AppleEventCode</key>
	<string>pvct</string>
	<key>ReadOnly</key>
	<string>YES</string>
	<key>Type</key>
	<string>NSNumber</string>
</dict>

.scriptTerminology:

<key>numberOfVisits</key>
<dict>
	<key>Description</key>
	<string>The number of times the bookmark has been visited.</string>
	<key>Name</key>
	<string>visit count</string>
</dict>

Those look equivalent to me :-(  Peter, any ideas here?

Comment 5

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

Attachment: Use "integer" instead of "number"

OK, setting that to "integer" instead of "number" seems to work.  I note that "count" also uses "integer" in the .sdef (only window ID and index use "number", though there's a note in our .sdef that index should be "integer", and the current skeleton.sdef has both using "integer").

I'm not clear on the significance of the difference or why compiling the .sdef into .script* works on branch; luck (everything in the .script* seems to be NSNumber)?

Comment 6

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

Eiichi, can you test this patch on 10.4 to make sure that the AppleScript in comment 0 works on 10.4 with this patch?

Comment 7

[Eiichi]

(In reply to comment #6)
> Eiichi, can you test this patch on 10.4 to make sure that the AppleScript in
> comment 0 works on 10.4 with this patch?

I confirm that the AppleScript in comment 0 works fine on 10.4 with the patch.

Comment 8

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

(Unsurprisingly in afterthought, this is the same fix as for bug 467697.)

Comment 9

[Lisa Thompson]

Do you think that would work for the id property of windows? Cuz that's never worked either, same "AppleEvent handler failed" message. (I know, one issue per bug, sorry; I'll file it separately if you want.)

Comment 10

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

(In reply to comment #9)
> Do you think that would work for the id property of windows? Cuz that's never
> worked either, same "AppleEvent handler failed" message.

Yes, I expect it should fix it.  I intended to test that and wrap up our "spurious number" problem once and for all before seeking review on this patch.  It's just not at the top of my to-do list this week due to more pressing issues :-(  If you'd like to edit your .sdef and test, please do :-)

Comment 11

[Lisa Thompson]

I was going to say I didn't know how to edit an .sdef, but apparently it's just text because I made the changes in TextWrangler and it worked. Specifically, I changed both occurrences of type="number" to type="integer" associated with window id and visit count, and scripts accessing those properties now behave as expected. Tested with today's 2.0b4pre from Script Editor, script menu, and toolbar script.

Comment 12

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

(In reply to comment #11)
> I was going to say I didn't know how to edit an .sdef, but apparently it's just
> text because I made the changes in TextWrangler and it worked.

Sorry, you've gotten so good at filing these bugs that I just assumed you knew the back-end implementation details, too ;)

Thanks for testing that; I'll update the patch and push it through!

Comment 13

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

Attachment: Fixes visit count and window id

This fixes both bookmarks visit count and the Core suite window id by using "integer" instead of "number" (the latter as the Core suite currently does in Skeleton.sdef).

Comment 14

[Stuart Morgan]

Comment on attachment 397493 [details] [diff] [review]
Fixes visit count and window id

sr=smorgan

Comment 15

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

Checked in on cvs trunk and CAMINO_2_0_BRANCH.