Closed
Bug 1100284
Opened 10 years ago
Closed 9 years ago
Loop rooms shouldn't allow opening the same room more than once
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35 verified, firefox36 verified)
backlog | Fx35+ |
People
(Reporter: standard8, Assigned: mikedeboer)
References
Details
(Whiteboard: [rooms])
Attachments
(2 files, 2 obsolete files)
3.67 KB,
patch
|
mikedeboer
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
1.65 KB,
patch
|
MattN
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Currently, if I select a room twice in the panel, then the room opens and connects to itself. Really, we should focus the existing window.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 36.3
Points: --- → 3
Flags: qe-verify+
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Assignee | ||
Comment 2•10 years ago
|
||
Well, this seems to get us there, except for detached windows... but that's a bug in Social code.
Attachment #8524739 -
Flags: review?(standard8)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8525429 -
Flags: review?(mixedpuppy)
Updated•10 years ago
|
Attachment #8525429 -
Flags: review?(mixedpuppy) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8524739 [details] [diff] [review] Patch 1: use a more canonical window ID for chat windows On the outset it looks fine, but I'm not really familiar with this code, probably better to get someone else to review. My initial question is whether gLastWindowId is still necessary at all.
Attachment #8524739 -
Flags: review?(standard8) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
Fixed a whoopsie... Carrying over r=mixedpuppy. Thanks much, Shane!
Attachment #8525429 -
Attachment is obsolete: true
Attachment #8525970 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8524739 -
Flags: review?(standard8)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8524739 -
Attachment is obsolete: true
Attachment #8524739 -
Flags: review?(standard8)
Attachment #8525975 -
Flags: review?(standard8)
Updated•10 years ago
|
backlog: --- → Fx35+
Priority: -- → P1
Whiteboard: [rooms]
Comment 7•10 years ago
|
||
Comment on attachment 8525975 [details] [diff] [review] Patch 1.1: use a more canonical window ID for chat windows Review of attachment 8525975 [details] [diff] [review]: ----------------------------------------------------------------- This seems reasonable. rs=me ::: browser/components/loop/MozLoopService.jsm @@ +761,5 @@ > openChatWindow: function(conversationWindowData) { > // So I guess the origin is the loop server!? > let origin = this.loopServerUri; > + // Try getting a window ID that can (re-)identify this conversation, or resort > + // to a globally unique one as a lasy resort. Typo: s/lasy/last/ @@ +765,5 @@ > + // to a globally unique one as a lasy resort. > + // XXX We can clean this up once rooms and direct contact calling are the only > + // two modes left. > + let windowId = ("contact" in conversationWindowData) ? > + conversationWindowData.contact._guid : Perhaps the guid should become a public property (i.e. drop the _ prefix) in the future?
Attachment #8525975 -
Flags: review?(standard8) → review+
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22; away Nov. 14-19) from comment #7) > Typo: s/lasy/last/ Fixed. > Perhaps the guid should become a public property (i.e. drop the _ prefix) in > the future? Myeah, that's prolly a good idea. But that would require a DB migration, though. :/ Pushed to fx-team as: https://hg.mozilla.org/integration/fx-team/rev/a7621853f055 https://hg.mozilla.org/integration/fx-team/rev/a46accc80752
Backed out for xpcshell bustage: remote: https://hg.mozilla.org/integration/fx-team/rev/7228e1d598ee remote: https://hg.mozilla.org/integration/fx-team/rev/6fec67fdc58b https://treeherder.mozilla.org/ui/logviewer.html#?job_id=1257652&repo=fx-team
Flags: needinfo?(mdeboer)
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e8f45cf7b043 https://hg.mozilla.org/integration/fx-team/rev/c773c6be9270 Updated to resolve xpcshell failure (_guid was undefined, so fallback to last window id) r=abr on IRC Tested locally - lets you in twice without the patch; ignores second attempt to join with this patch. Mike: just double-check when you get a chance. (I added "|| gLastWindowId++" after "....contacts._guid")
And back out again... https://hg.mozilla.org/integration/fx-team/rev/1a9ce687110d https://treeherder.mozilla.org/ui/logviewer.html#?job_id=1262064&repo=fx-team
Flags: needinfo?(rjesup)
Comment 12•10 years ago
|
||
fixed second xpcshell bustage: https://hg.mozilla.org/integration/fx-team/rev/33a80305f1b4 https://hg.mozilla.org/integration/fx-team/rev/f5c0481fe56b try: https://tbpl.mozilla.org/?tree=Try&rev=f4c573fed0fa
Flags: needinfo?(rjesup)
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/33a80305f1b4 https://hg.mozilla.org/mozilla-central/rev/f5c0481fe56b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 14•9 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #13) > https://hg.mozilla.org/mozilla-central/rev/33a80305f1b4 > https://hg.mozilla.org/mozilla-central/rev/f5c0481fe56b Is test coverage sufficient or will this need further test coverage?
Comment 15•9 years ago
|
||
Comment on attachment 8525970 [details] [diff] [review] Patch 2.1: also support detached chat windows in duplicate chat window handling Approval Request Comment [Risks and why]: Loop rooms code needed for 35 pref-on Connecting a call to yourself is both confusing and useless (and causes other potential problems). [String/UUID change made/needed]: none
Attachment #8525970 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8525975 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Flags: needinfo?(mdeboer)
Updated•9 years ago
|
Attachment #8525970 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Attachment #8525975 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Comment 17•9 years ago
|
||
I was able to reproduce this issue on Firefox 36.a1 (2014-11-23) using Windows 7 x64. Verified fixed on latest Firefox 36.0a2 (2014-12-17) and Firefox 35 Beta 4 (20141216120925) using Windows 7 x64 and Ubuntu 14.04 x86.
You need to log in
before you can comment on or make changes to this bug.
Description
•