Closed Bug 805547 Opened 13 years ago Closed 13 years ago

[WindowManager][Communications] system message does not see the already opened window and creates a new one

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia
Platform:
x86_64
Linux
Type:
defect

Tracking

(blocking-basecamp:+)

Status:
RESOLVED INVALID
Project Flags:
blocking-basecamp +

People

(Reporter: etienne, Assigned: timdream)

References

Details

Probably an issue with the multiple entry points. STR: - launch the dialer - make an incoming phone call to the phone - end the call - show the card view Expected: - the card view shows only a Dialer app Actually - we have a Dialer app *and* a Communications app open It's pretty bad to open the dialer twice memory wise and having those 2 instances causes edge cases issues. Probably a blocker.
blocking-basecamp: --- → ?
blocking-basecamp: ? → ---
Well, looks like the WindowManager sees dialer/index.html and dialer/dialer.html#keyboard-view as needing two separate frames. But it's easily fixable for the Dialer by specifying dialer/dialer.html#keyboard-view in the manifest when registering for the system messages. So no rush :)
Why wouldn't index.html and dialer.html need two separate frames? I'm actually pretty unqualified to comment on this bug. I haven't touched window_manager.js in a long time, and don't know anything about the dialer or communication apps. But I'm cc'ing Tim, because I know he just fixed another window_manager.js bug relating to the clock being launched by a system message.
(In reply to David Flanagan [:djf] from comment #2) > Why wouldn't index.html and dialer.html need two separate frames? Thank you! This mistake was rendering this bug report pretty unreadable. I meant dialer/index.html and dialer/index.html#keyboard-view
(In reply to Etienne Segonzac (:etienne) from comment #3) > (In reply to David Flanagan [:djf] from comment #2) > > Why wouldn't index.html and dialer.html need two separate frames? > > Thank you! This mistake was rendering this bug report pretty unreadable. > > I meant dialer/index.html and dialer/index.html#keyboard-view Sounds like a quirk on URL matching in platform; but I can look deeper by console.log() some mozChromeEvent
Assignee: nobody → timdream+bugs
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Priority: -- → P3
Increasing priority based on partner request.
Priority: P3 → P2
Note that this issue should not happen anymore with the Dialer/Communication app (we're working around it with a strict manifest policy).
It is still happening for the communications app. This is a critical bug.
Priority: P2 → P1
Faramarz, please make sure that severe regressions and logic bugs like this one (starting apps twice) get resolved immediately. This is day 5 for this bug. Thats not a good track record. For this kind of bug other work should be put aside if needed. This is the definition of a critical dogfooding blocker.
:timdream bumped to "critical" based on comment from :gal. please prioritize this higher on your list of to-do.
Severity: normal → critical
(In reply to Faramarz (:faramarz) from comment #10) > :timdream > bumped to "critical" based on comment from :gal. please prioritize this > higher on your list of to-do. Good morning!
It looks like Etienne have worked around this bug with https://github.com/mozilla-b2g/gaia/commit/cb0455a1f8a8c7e2ee500bda5357e9dc5f343d1d#L3R66 That's why it was not prioritized.
The work around doesn't work on trunk.
(In reply to Andreas Gal :gal from comment #13) > The work around doesn't work on trunk. It works for me on Otoro with the exact same STR. I am writing a comment to break down the cause of this right now.
I can only conclude that this is not a bug but rather a confusion or undefined behavior of entry_points and system message on how to handle URL with hash (#). Window Manager currently identifies app frames with their "origin". With the introduction of the entry points, the "origin" have since diverged from the meaning of "origin" as in same-origin policy. If the "origin" is the same, window manager will impose one-window-per-app-entry-point policy and not to launch another frame. The "origin" string used to identify the launched dialer app, is "app://communications.gaiamobile.org/dialer/index.html#keyboard-view", specified by its launch path. [1] The launch path is being used to compose the "origin" string at here [2]. This all happens when the user launches dialer app (or, strictly speaking, "Dialer" entry point of the Communication app) from home screen (the first step in STR in comment 0) When there is an incoming call (STR step 2 in comment 0), window manager receives another mozChromeEvent; but this time, the launch path contained in the event is from here [3] instead. If one URL comes with '#keyboard-view' and the other doesn't, it's totally legit for window manager to think they are two different entries of the same app, thus launches another Communication app frame. The bottom line is, we need to define the way we identify entry points [4]. It currently does the URL matching in a hacky way, which compares the name of the entry point with the first level path [4,#L1172], and then exact launch path. HOWEVER, before that, it would strip out the query string part [5]. I don't have a proposed fix here. I don't want to "correct" the behavior again by strip the hash part of the URL before comparison. Is there a spec available for mozApps API regarding entry points? What if there is an app, with just one html, intend to use hashes to specify different entry points? Why are we stripping query string? The change mentioned in comment 12, from my test, fix the behavior mentioned in comment 0. Unless I am explicitly told that URL-to-app-frame matching should NOT consider the hash part of the URL (and the query string part), and the people who told me that can say this will not add more confusion and/or regression, the behavior should not be considered a bug and the comment 12 change is the exact way how apps should tell Gaia which frame should you launch for handling the system message. [1] https://github.com/mozilla-b2g/gaia/blob/a792e90e1d96a7e079a9981d2f8b9bef659ed7f5/apps/communications/manifest.webapp#L12 [2] https://github.com/mozilla-b2g/gaia/blob/a792e90e1d96a7e079a9981d2f8b9bef659ed7f5/apps/system/js/window_manager.js#L1174 [3] https://github.com/mozilla-b2g/gaia/blob/a792e90e1d96a7e079a9981d2f8b9bef659ed7f5/apps/communications/manifest.webapp#L70 [4] https://github.com/mozilla-b2g/gaia/blob/a792e90e1d96a7e079a9981d2f8b9bef659ed7f5/apps/system/js/window_manager.js#L1155 [5] https://github.com/mozilla-b2g/gaia/blob/a792e90e1d96a7e079a9981d2f8b9bef659ed7f5/apps/system/js/window_manager.js#L1167
(In reply to Andreas Gal :gal from comment #13) > The work around doesn't work on trunk. Which trunk? Can't reproduce the bug with a gaia (master) & aurora from right now.
Should the platform check that the system message handler listed in the manifest is one of the listed (apps launch path + entry points launch paths) paths? Also, if I am not wrong, all the hacks that Tim mentioned should be solved once we solve Bug 796629.
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #18) > Also, if I am not wrong, all the hacks that Tim mentioned should be solved > once we solve Bug 796629. Policy decisions can not be fixed by code change. We would still need to make a decision on how to match URLs before proceed with a fix. (In reply to Fernando Jiménez Moreno [:ferjm] from comment #18) > Should the platform check that the system message handler listed in the > manifest is one of the listed (apps launch path + entry points launch paths) > paths? Just like web activities, I believe app authors should be free to assign a URL that is only reachable with system message.
Andreas, is STR in comment 0 still happens to you? If not, we should INVALID this bug and move the URL matching discussion to bug 796629 (which is non-blocking)
Flags: needinfo?(gal)
No device to test this. Why do you need me to test this? If it doesn't happen for you, assume its fixed.
Flags: needinfo?(gal)
(In reply to Andreas Gal :gal from comment #21) > No device to test this. Why do you need me to test this? If it doesn't > happen for you, assume its fixed. Sure. Thanks.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.