Adium

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#15234 closed defect (fixed)

AppleScript parameter "at location" broken for chats

Reported by: Aragos Owned by: earthmkii
Milestone: Adium 1.5 Component: Adium Core
Version: 1.4.2b2 Severity: normal
Keywords: applescript make move Cc:
Patch Status: Accepted

Description

Summary

The applescript "make" command parameter "at location" does not work. The same is true for the "to" parameter of the "move command.

Steps to reproduce

Write a simple applescript and execute:

tell application "Adium"
	set user to first contact whose (display name starts with "Foo")
	
	if not (exists (chats whose contacts contains user)) then
		if not (exists (first chat window)) then
			tell account of user to (make new chat with contacts {user} with new chat window)
		else
			set existing_window to first chat window
			set existing_chat to last chat of chat window

			-- using the problematic command:
			tell account of user to (make new chat with contacts {user} at after existing_chat)
		end if
	end if
end tell

Expected results

I expect the new chat to appear in the tabs all the way to the right.

Actual results

Adium crashes.

Regression

None I know of.

Notes

The problem originates from misbehaviour of chats in the AppleScript framework: In AIChat.m:892 the chat's container is set to nil where it should be the containing chat window. In AIAccount.m:1118 the code expects the insertioncontainer for the chat to be a chat window but receives nil instead.

The attached patch also addresses two other applescript issues.

Attachments (4)

applescript.patch (5.1 KB) - added by Aragos 8 years ago.
AppleScript patch (fixes at/to parameter, adds "in window" parameter and "become active" command)
applescript-will-patch.diff (4.0 KB) - added by kbotc 8 years ago.
I didn't test it, but I went ahead and created a patch that will apply
applescript_15.patch (7.2 KB) - added by Aragos 8 years ago.
Aidum 1.5 compatible version of this patch, tested
copyright.patch (269 bytes) - added by Aragos 8 years ago.
copyright entry

Download all attachments as: .zip

Change History (19)

Changed 8 years ago by Aragos

AppleScript patch (fixes at/to parameter, adds "in window" parameter and "become active" command)

comment:1 Changed 8 years ago by Robby

  • Milestone set to Adium 1.5
  • Patch Status changed from Initially Included to Needs Dev Review

comment:2 Changed 8 years ago by robotive

  • Patch Status changed from Needs Dev Review to Needs Changes by Author

comment:3 Changed 8 years ago by David Munch

Ticket #15235 has been marked as a duplicate of this ticket.

comment:4 Changed 8 years ago by Aragos

Ah, totally forgot about this submission... what changes are required?

comment:5 Changed 8 years ago by robotive

Sorry mate, my repo is heavy modified atm, and i don't want to break something. Just try to apply the patch, iirc it'll fail, or the patch won't compile. i think it'll be an easy fix for you.

Thanks a lot.

comment:6 Changed 8 years ago by Robby

For me the patch fails to apply:

patching file Frameworks/Adium Framework/Source/AIAccount.m
Hunk #2 FAILED at 1111
1 out of 2 hunks FAILED -- saving rejects to file Frameworks/Adium Framework/Source/AIAccount.m.rej
abort: patch failed to apply

Changed 8 years ago by kbotc

I didn't test it, but I went ahead and created a patch that will apply

comment:7 Changed 8 years ago by kbotc

  • Patch Status changed from Needs Changes by Author to Needs Dev Review

comment:8 Changed 8 years ago by Aragos

I'm currently investigating this. I moved the patch from 1.4.x to 1.5 but it appears to have lost some functionality around window management. Once that's fixed I'll upload a new version that should work against 1.5 HEAD.

Changed 8 years ago by Aragos

Aidum 1.5 compatible version of this patch, tested

comment:9 Changed 8 years ago by Aragos

I finally managed to figure out what was breaking the applescript support in 1.5 (unsafe use of hashes) and fixed that. The new patch succeeds for all test applescripts I have at HEAD.

comment:10 Changed 8 years ago by earthmkii

  • Owner set to earthmkii
  • Status changed from new to assigned

Will review this patch

comment:11 Changed 8 years ago by Stephen Holt <sholt@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

(In 6e7db5aeabcc) Reviewed and acepted patch from Aragos that fixes some AppleScript issues. Fixes #15234.

comment:12 Changed 8 years ago by earthmkii

Aragos, can you please include a patch for Copyright.txt with your name (or, if you prefer, pseudonym) in alphabetical order? Thanks.

comment:13 Changed 8 years ago by Robby

  • Patch Status changed from Needs Dev Review to Accepted

Changed 8 years ago by Aragos

copyright entry

comment:14 Changed 8 years ago by Robert Vehse

(In e3f4492a25db) Credit Peter Schmitt for his patch. Refs #15234.

Also some changelogging.

comment:15 Changed 8 years ago by Robby

Ticket #15236 has been marked as a duplicate of this ticket.

Note: See TracTickets for help on using tickets.