Adium

Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#7401 closed defect (fixed)

incorrect message classes

Reported by: mathuaerknedam Owned by: Catfish_Man
Milestone: Adium 1.3 Component: Message View
Version: Severity: normal
Keywords: Cc:
Patch Status:

Description (last modified by David Smith)

as of 1.1b3, date seperators, status messages, and autoreplys all have a %messagecClasses% of "incoming". Incoming and outgoing messages. <second section snipped as noted in comments>

Change History (26)

comment:1 Changed 13 years ago by David Smith

Owner: set to David Smith
Status: newassigned

comment:2 Changed 13 years ago by David Smith

Milestone: Adium X 1.1

comment:3 Changed 13 years ago by mathuaerknedam

The first half of the ticket is correct, but this part: "Also, old messages ("context" in html)only appear to have an incoming/outgoing class, and omit the "history" class." can be ignored. Sorry about that.

comment:4 Changed 13 years ago by David Smith

Description: modified (diff)

comment:5 Changed 13 years ago by David Smith

Milestone: Adium X 1.1Needs feedback from users

I can't reproduce the first section of this either.

comment:6 Changed 13 years ago by mathuaerknedam

Hm, I'm not sure how I managed to munge the description like that. It should say:

As of 1.1b3, date separators, status messages, and autoreplys (probably others) all have "incoming" prepended to their list of %messagecClasses%. For example, the date separator's %messageClasses% is "incoming status date_separator event".

I've been checking the %messageClasses% by adding "title="%messageClasses%" to the html of my messagestyle. This is also already in SO2's status messages, so it can be verified there as well.

comment:7 Changed 13 years ago by David Smith

I'm using the Web Inspector to investigate, and I see a mix of incoming and outgoing as expected.

comment:8 Changed 13 years ago by mathuaerknedam

aha!! It might be good to update CreatingMessageStyles to document that messageClasses requires Safari 3.

I've tried opening index.html (from inside SO2) in Safari and using the WEb Inspector, but I'm not doing something right because I'm not seeing the values and I'm not sure how they'd get set outside of Adium anyhow.

Should status messages of any sort, but especially date_separators, be either incoming or outgoing? I'd think the date_seperators should be incoming/outgoing agnostic.

Maybe I should just stop poking at things I don't understand. :)

comment:9 Changed 13 years ago by David Smith

I was using the web inspector from inside Adium; You're correct that it won't get set if you load it up in Safari.

There's a hidden preference you can set on any webkit using app to enable the inspector.

comment:10 Changed 13 years ago by mathuaerknedam

From IRC...

[13:13] mathuaerknedam: Catfish_Man: So now that I've gotten thing mostly behaving like expected, I'm wondering if a date separator should ever have a messageClasses of incoming. [13:13] Catfish_Man: mathuaerknedam: it probably shouldn't; I've been considering how to remove incoming/outgoing from things that don't have it [13:13] Catfish_Man: I may make only AIContentMessage and subclasses have that [13:13] Catfish_Man: which would eliminate status and events

[16:07] mathuaerknedam: I've also been thinking about the incoming status messages... [16:08] mathuaerknedam: Having incoming as a messageClasses for some status messages does make sense, since some are very buddy-specific. It seems to make sense (to me) for there to be some way to distinguish between user-specific status/events. [16:08] mathuaerknedam: It may not be worth the work, but there's definitely some logic to it. [16:09] mathuaerknedam: It'd be cool if I could consecutivize all of Alice's status changes without mixing up Bob's. [16:09] Catfish_Man: mmm good idea

comment:11 Changed 13 years ago by mathuaerknedam

From IRC...

[13:13] mathuaerknedam: Catfish_Man: So now that I've gotten thing mostly behaving like expected, I'm wondering if a date separator should ever have a messageClasses of incoming.
[13:13] Catfish_Man: mathuaerknedam: it probably shouldn't; I've been considering how to remove incoming/outgoing from things that don't have it
[13:13] Catfish_Man: I may make only AIContentMessage and subclasses have that
[13:13] Catfish_Man: which would eliminate status and events

[16:07] mathuaerknedam: I've also been thinking about the incoming status messages...
[16:08] mathuaerknedam: Having incoming as a messageClasses for some status messages does make sense, since some are very buddy-specific. It seems to make sense (to me) for there to be some way to distinguish between user-specific status/events.
[16:08] mathuaerknedam: It may not be worth the work, but there's definitely some logic to it.
[16:09] mathuaerknedam: It'd be cool if I could consecutivize all of Alice's status changes without mixing up Bob's.
[16:09] Catfish_Man: mmm good idea

comment:12 Changed 13 years ago by Robert

Milestone: Needs feedback from usersGood idea for "later"
Version: 1.1b31.2.4b2

comment:13 Changed 13 years ago by mathuaerknedam

With the help of Robby, I've verified that this does still seem to be a problem and compiled a list of classes applied to status messages, as of 1.2.4b2.

date_separator: "incoming status date_separator event"

User went away: "incoming status away"

Encrypted OTR chat initiated. user's identity not verified: "incoming status encryptionStartedUnverified event"

Encrypted OTR chat initiated: "incoming status encryptionStarted event"

User is no longer using encryption: "incoming status encryption event"

User has left the conversation: "incoming status libpurpleMessage event"

User disconnected: "incoming status offline"

User connected: "incoming status online"

Away message: "incoming status away_message"

Your message was not sent. Either end your private conversation, or restart it: "incoming status encryption event"

And using the Adium and the IRC plugin built from r22963:

IRC topic: "incoming status purple event"

IRC users entering/leaving: "incoming status libpurpleMessage event"

comment:14 Changed 12 years ago by David Smith

Resolution: fixed
Status: assignedclosed

(In [23296]) Remove the 'status' and 'incoming' styling classes from AIContentEvent, since neither really applies. Fixes #7401

comment:15 Changed 12 years ago by David Smith

Milestone: Good idea for "later"Adium X 1.3

comment:16 Changed 12 years ago by mathuaerknedam

Resolution: fixed
Status: closedreopened

Unfortunately, this doesn't quite look fixed. Using r23311, doing inspect element on the two status messages in the messages prefpane preview reveals that they still have the incoming class. Or is the intention for status messages to retain the incoming class?

comment:17 Changed 12 years ago by David Smith

Status messages make sense as incoming to me, since your status messages are never shown. If that's a problem I can move the fix up one level, but conceptually it's still a "from you to me" thing, i.e. incoming.

comment:18 Changed 12 years ago by mathuaerknedam

I think it's fine, but the current paradigm of classifying everything as in/out/status is pretty ingrained, even down to the html files that are used to generate the messages. messageClasses and optional files are beginning to change this, but status.html is still required for things that are no longer classified as "status". Considering that nobody but hawkman and myself are using messageClasses (I think), I think 1.3 would be a great time to make a root-level content.html the only required html file. I think this would help authors make the jump to thinking about messageClasses with the flexibility of tagging rather than the structure of a hierarchy.

comment:19 Changed 12 years ago by mathuaerknedam

Oh, and of course now there are keywords, like %status% and %statusSender%, that are now confusingly named since they apply to things that aren't classified as status.

comment:20 Changed 12 years ago by David Smith

Resolution: fixed
Status: reopenedclosed

(In [23436]) Removing direction style classes from status messages, snce they are pretty useless. Fixes #7401 again, although a better fix may eventually happen once I determine what a better fix would actually look like

comment:21 Changed 12 years ago by mathuaerknedam

Resolution: fixed
Status: closedreopened

Evan asked me to open tickets for any instance of a change to Adium breaking existing messagestyles. In this case, I'm simply reopening this one, because [23296] removed the status class from events. This broke all of my messagestyles since I had successfully used "status" to select both status and event. At this point, All of my messagestyles but aNon have been updated, and for aNon it's only a matter of my uploading a new xtra (and taking new screenshots, etc).

I suspect that I'm the only messagestyle author affected by this (that's what I get for taking advantage of features as soon as they're available), so I won't feel bad if it's closed with "we shouldn't break things, but we won't undo it this time, sucks to be you". :)

comment:22 Changed 12 years ago by Evan Schoenberg

Did it break your message styles because you were making use of a documented aspect of the message style which has now been broken? Or did it break them because you were making use good use of a bug which was not per the documentation?

comment:23 Changed 12 years ago by mathuaerknedam

Technically, I was making good use of what could be considered a bug in so far as the behavior was not consistent with the documentation. However, as I mentioned in Comment 18, the "bug" was consistent with the classing mechanism virtually every messagestyles uses (fixed classes statically assigned in html files). Most notably status.html, which was and still is *required* for the content of status (and not event) messages.

I'm not really complaining abut making a distinction between events and status messages. But it's definitely confusing to say (via messageClasses) that events are not status messages, but you still have to use status.html to control them.

comment:24 Changed 12 years ago by David Smith

Milestone: Adium X 1.3Adium X 1.3.x

Not a blocker

comment:25 Changed 11 years ago by mathuaerknedam

Resolution: fixed
Status: reopenedclosed

Unfortunate that status.html is not misnamed for most cases that use it, but that's really a different issue than the one this ticket was opened for. Time moves on.

comment:26 Changed 11 years ago by Jordan

Milestone: Adium 1.3.xAdium 1.3
Note: See TracTickets for help on using tickets.