Closed Bug 828182 Opened 11 years ago Closed 11 years ago

identifier close mozChromeEvent should let gaia know which frame it's fired on.

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.0 fixed)

RESOLVED FIXED
blocking-b2g tef+
Tracking Status
b2g18 --- fixed
b2g18-v1.0.0 --- fixed

People

(Reporter: alive, Assigned: jedp)

References

Details

(Whiteboard: [triaged:1/15][qa-], QARegressExclude)

Attachments

(1 file)

I see https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/identity.js#L67
and learns the fact that if we have two identifier frame at the same time, we may not know which frame is closing. It's a pain to maintain an queue in gaia, so I think the simpler and best way is attach frame info in close event.

Jason, please help to move to the correct component if I am wrong. 
Thanks!
Component: Identity → Gaia::System
Product: Core → Boot2Gecko
Version: Trunk → unspecified
Component: Gaia::System → Identity
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Talked with Alive and Ferjm in IRC about this:

The impact of this bug is a potential risk that closing one trusted UI with persona up when two persona dialogs up will close the wrong dialog.

It's edge case enough though not to block, as I don't expect multiple dialogs up at the same time to happen that often.
Blocks: 830358
Yikes. This is causing a nasty hang right now with the trusted UI. Jed - Can you investigate?
blocking-b2g: --- → tef?
tracking-b2g18: --- → ?
Flags: needinfo?(jparsons)
I can investigate, yes.  Are the STR what you describe in Bug 830358?
Flags: needinfo?(jparsons)
(In reply to Jed Parsons [:jparsons] from comment #3)
> I can investigate, yes.  Are the STR what you describe in Bug 830358?

Yup, that's correct.
Blocks: basecamp-id
Spoke with Fernando in IRC and got caught up on what you all had already discussed.  I had worried about this in the past (it's actually my comment at the top of the open function :)  Sounds like the fix is simply to require the caller of close() to pass its chromeEventId as it does on open()

fwiw, I think this is incorrectly filed as an identity bug.  Since Trusty UI is so crucial, I think it should have its own component in gaia.

Assigning to me
Assignee: nobody → jparsons
blocking-b2g: tef? → tef+
The trusty UI needs tests, badly.  I've spun off bug 830528 to track the need for tests.  Any tests I put together for this feature I will attach to that bug, since just getting the test suite started for TUI seems like an entirely separate issue.
I've tested this with Identity, but I don't have a good test environment for Marketplace.  Please ensure that I've done the right thing with payment.js.  Thanks!
Attachment #702061 - Flags: review?(ferjmoreno)
Component: Identity → Gaia::System
Product: Core → Boot2Gecko
Version: Trunk → unspecified
Thanks Jed. I'll take a look at this tonight.
Thanks, Fernando.  

I'll keep an eye on Bug 813811; as soon as the PR for that lands in gaia, I'll update this PR.  We're bound to have a couple of minor conflicts.
Comment on attachment 702061 [details]
Use chromeEventId to select which frame to close

r=me

I've left a comment in the PR.

Don't worry about Bug 813811. I'll merge this first as it was submitter in first place, so you don't need to rebase your patch :).
Attachment #702061 - Flags: review?(ferjmoreno) → review+
Ok.  Thanks very much, Fernando; I'll wait until this is merged.
Whiteboard: [triaged:1/15]
tracking-b2g18: ? → ---
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [triaged:1/15] → [triaged:1/15][qa-]
Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.
No need to make a Test Case in Moztrap for this issue.
Flags: in-moztrap-
Whiteboard: [triaged:1/15][qa-] → [triaged:1/15][qa-], QARegressExclude
Cannot verify, need steps to blackbox test this issue.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: