Closed
Bug 1079811
Opened 10 years ago
Closed 10 years ago
A new call won't start if the outgoing call window is opened (showing feedback or retry/cancel)
Categories
(Hello (Loop) :: Client, defect, P2)
Tracking
(firefox33 unaffected, firefox34+ verified, firefox35+ verified, firefox36+ verified)
backlog | Fx34+ |
People
(Reporter: pauly, Assigned: rgauthier)
References
Details
Attachments
(1 file, 2 obsolete files)
7.62 KB,
patch
|
standard8
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
A new call won't start if the outgoing call window is opened (showing feedback or retry/cancel)
Comment 1•10 years ago
|
||
Shell -- can we add this to our trello?
Flags: needinfo?(sescalante)
Priority: -- → P2
Updated•10 years ago
|
Flags: qe-verify?
Flags: firefox-backlog+
Comment 3•10 years ago
|
||
The work here is to port the existing calls/lines of code: navigator.mozLoop.releaseCallData(this.props.conversation.get("callId")); to the conversationStore in equivalent places.
Updated•10 years ago
|
backlog: --- → Fx36+
Updated•10 years ago
|
backlog: Fx36+ → Fx35+
Flags: qe-verify? → qe-verify+
QA Contact: anthony.s.hughes → paul.silaghi
Updated•10 years ago
|
backlog: Fx35+ → Fx34+
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8506153 -
Flags: feedback?(standard8)
Comment 6•10 years ago
|
||
Comment on attachment 8506153 [details] [diff] [review] WIP: Bug 1079811 fix outgoing calls overlaping Review of attachment 8506153 [details] [diff] [review]: ----------------------------------------------------------------- This good, I've just tested this and it seems to do the right thing. All you need now is to extend the existing tests for it. You should be able to base the tests on the ones where we check for closing the websocket already. ::: browser/components/loop/content/shared/js/conversationStore.js @@ +374,5 @@ > this._websocket.close(); > delete this._websocket; > } > + > + // The internal callId is different from this.get("callId"); Please make this an XXX and reference bug 1084228.
Attachment #8506153 -
Flags: feedback?(standard8) → feedback+
Comment 7•10 years ago
|
||
[Tracking Requested - why for this release]: Tracking fix for uplift. We should have a patch for review today.
tracking-firefox34:
--- → ?
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8506153 -
Attachment is obsolete: true
Attachment #8506901 -
Flags: review?(standard8)
Comment 9•10 years ago
|
||
Comment on attachment 8506901 [details] [diff] [review] Bug 1079811 fix outgoing calls overlaping Review of attachment 8506901 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/content/shared/js/conversationStore.js @@ +408,5 @@ > } > + > + // XXX: The internal callId is different from > + // this.get("callId"), see bug 1084228 for more info. > + var callId = window.location.href.match(/\#outgoing\/(.*)/)[1]; Please use something like: var locationHash = new loop.shared.utils.Helper().locationHash(); var callId = locationHash.match( ... )[1]; We've created that in the helper to make it easier to stub for tests... ::: browser/components/loop/test/shared/conversationStore_test.js @@ +124,5 @@ > > describe("#connectionFailure", function() { > beforeEach(function() { > store._websocket = fakeWebsocket; > + window.location.hash = "outgoing/42"; Please stub the helper#locationHash function as we do elsewhere. This saves changing the side effects of changing the hash of the test page.
Attachment #8506901 -
Flags: review?(standard8) → review-
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8506901 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8506936 -
Flags: review?(standard8)
Comment 11•10 years ago
|
||
Comment on attachment 8506936 [details] [diff] [review] Bug 1079811 fix outgoing calls overlaping Review of attachment 8506936 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. r=Standard8
Attachment #8506936 -
Flags: review?(standard8) → review+
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/808971467f9f
Iteration: --- → 36.1
Target Milestone: --- → mozilla36
Updated•10 years ago
|
status-firefox33:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox35:
--- → +
tracking-firefox36:
--- → +
https://hg.mozilla.org/mozilla-central/rev/808971467f9f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
Tested on Windows and Win32 Nightly 10/18; works!
Comment 15•10 years ago
|
||
Comment on attachment 8506936 [details] [diff] [review] Bug 1079811 fix outgoing calls overlaping Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: Inability to make/receive new calls while old call window is still open [Describe test coverage new/current, TBPL]: Tested manually on multiple OS's in Nightly. [Risks and why]: Needed for feature release [String/UUID change made/needed]: none
Attachment #8506936 -
Flags: approval-mozilla-beta?
Attachment #8506936 -
Flags: approval-mozilla-aurora?
Comment 16•10 years ago
|
||
Comment on attachment 8506936 [details] [diff] [review] Bug 1079811 fix outgoing calls overlaping Thanks for testing jesup. Approved for Aurora. If everything goes well with your testing on Aurora on Sunday, we'll get this uplifted for beta2.
Attachment #8506936 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•10 years ago
|
Updated•10 years ago
|
Comment 18•10 years ago
|
||
Tested and good on Aurora nightly build; windows and linux.
Comment 20•10 years ago
|
||
Comment on attachment 8506936 [details] [diff] [review] Bug 1079811 fix outgoing calls overlaping Previously approved offline. Adding approval to the bug.
Attachment #8506936 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 21•10 years ago
|
||
Marking verified based on Randell's testing, though this will still need verification in Beta once it uplifts.
Reporter | ||
Comment 22•10 years ago
|
||
I'm not sure if this is expected, but I can't make/receive another call if I have 4 outgoing call windows open in the retry/cancel state.
Flags: needinfo?(rgauthier)
Comment 23•10 years ago
|
||
(In reply to Paul Silaghi, QA [:pauly] from comment #22) > I'm not sure if this is expected, but I can't make/receive another call if I > have 4 outgoing call windows open in the retry/cancel state. Pauly -- can you open a new bug about this? Having 4 open outgoing call windows in the retry/cancel state is an edge case no one had tried. Thanks.
Flags: needinfo?(paul.silaghi)
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(paul.silaghi)
Updated•10 years ago
|
Flags: needinfo?(rgauthier)
Reporter | ||
Comment 24•10 years ago
|
||
Verified fixed FF 34b2 Win 7
You need to log in
before you can comment on or make changes to this bug.
Description
•