Closed
Bug 955490
Opened 10 years ago
Closed 10 years ago
Add data to 'conversation-loaded' notification to let observers know that the conversation was moved
Categories
(Instantbird Graveyard :: Conversation, enhancement)
Instantbird Graveyard
Conversation
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: benediktp, Assigned: benediktp)
References
Details
Attachments
(1 file)
1.35 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2053 at 2013-07-15 07:36:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Depends on: 954607
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2053 as attmnt 2597 at 2013-07-15 07:36:00 UTC *** This is the follow up to bug 954607 (bio 1175) to let observers know that the conversation-loaded event belongs to a conversation that was shown in a different window before. They might want to react differently in this case, e.g. by *not* adding things to the conversation content. The MathJax-add-on could make use of this as far as I know. Tested with an own extension (Input history) by reporting when it was attached to a conversation that was carrying this flag in the data parameter of the notification. No problems expected as the data parameter of the "conversation-loaded" notification doesn't seem to be used from anywhere else yet.
Attachment #8354366 -
Flags: review?(clokep)
Comment 2•10 years ago
|
||
*** Original post on bio 2053 at 2013-07-15 09:18:01 UTC *** Thanks for the followup! Does the data have to be a string? If not, I think I would prefer it if it was simply a boolean (true) in this case, as there is no data set in the other circumstances that conversation-loaded is sent.
Comment 3•10 years ago
|
||
Comment on attachment 8354366 [details] [diff] [review] Patch *** Original change on bio 2053 attmnt 2597 at 2013-07-15 10:26:26 UTC *** The data parameter does need to be a string: https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIObserverService#notifyObservers%28%29
Attachment #8354366 -
Flags: review?(clokep) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 4•10 years ago
|
||
*** Original post on bio 2053 at 2013-07-15 12:01:55 UTC *** (In reply to comment #1) > there is no data set in the other > circumstances that conversation-loaded is sent. I was wondering if we should put some data there for the other cases. I suspect it may be useful for add-ons to be able to differentiate if we are displaying a log in the log viewer, previewing a message theme in the pref window, or showing a conversation tab.
Comment 5•10 years ago
|
||
*** Original post on bio 2053 at 2013-07-15 12:09:28 UTC *** (In reply to comment #3) > (In reply to comment #1) > > > there is no data set in the other > > circumstances that conversation-loaded is sent. > > I was wondering if we should put some data there for the other cases. I suspect > it may be useful for add-ons to be able to differentiate if we are displaying a > log in the log viewer, previewing a message theme in the pref window, or > showing a conversation tab. It's already possible to differentiate by looking at the windowtype (eg https://bitbucket.org/aleth/mathjax-addon/src/e7ddef33088541b2df553c7dd18bc606384595fd/bootstrap.js?at=master#cl-110) but of course this would be nicer for add-on authors. Basically that would mean adding a windowtype check to http://lxr.instantbird.org/instantbird/source/chat/content/convbrowser.xml#838 Since the convbrowser is not supposed to care about what window it is in, I'm not sure if that should be done?
Comment 6•10 years ago
|
||
*** Original post on bio 2053 at 2013-07-15 14:05:47 UTC *** (In reply to comment #4) > Basically that would mean adding a windowtype check to > http://lxr.instantbird.org/instantbird/source/chat/content/convbrowser.xml#838 No, there are other ways. The way I had in mind would be to add an attribute in convbrowser that would be the 'type of conversation view' and would have 'conversation' as default value. The log viewer and preference windows would set this attribute to a different value when they initialize their convbrowsers. The convbrowser would just use the value of this attribute for the notification. The part that isn't clear to me is: is it worth doing, or given that no add-on seems to have desperately needed it yet, should we just take the current patch and add more stuff later if use cases appear?
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2053 at 2013-07-15 22:08:42 UTC *** We can morph or replace this bug to include the things that flo suggested. It sounds reasonable to pass such information with the notification and since there's no immediate need to land this as soon as possible, we could as well cover these other cases too at once.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 2053 at 2013-10-23 14:15:27 UTC *** Is this "checkin-needed" or should I go with flo's solution?
Comment 9•10 years ago
|
||
*** Original post on bio 2053 at 2013-10-23 16:23:46 UTC *** (In reply to comment #7) > Is this "checkin-needed" or should I go with flo's solution? It would be nice to check it in before 1.5 if you don't have the time for the more complicated idea before then.
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 10•10 years ago
|
||
*** Original post on bio 2053 at 2013-11-10 03:20:30 UTC *** http://hg.instantbird.org/instantbird/rev/e3fb5a5b56d1 Thanks! Can someone add documentation to the wiki about this please?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
Assignee | ||
Comment 11•10 years ago
|
||
*** Original post on bio 2053 at 2013-11-10 09:45:34 UTC *** Done.
Comment 12•10 years ago
|
||
*** Original post on bio 2053 at 2013-11-10 13:58:12 UTC *** (In reply to comment #10) > Done. To be clear, the details on https://wiki.instantbird.org/Instantbird:Notifications:trunk were updated!
You need to log in
before you can comment on or make changes to this bug.
Description
•