System Message API: System App fails to receive system messages (follow-up)

RESOLVED FIXED

Status

Firefox OS
Gaia
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Gene Lian (I already quit Mozilla), Assigned: Gene Lian (I already quit Mozilla))

Tracking

({regression})

unspecified
regression
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+)

Details

(Whiteboard: QARegressExclude)

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
This is a follow-up for fixing Bug 797803 that System App fails to receive system messages. Recently, it doesn't work again because of the check-in of Bug 801257 which did:

  -    if (msg.manifest != this._manifest)
  +    if (msg.manifest != this._manifest || msg.uri != this._uri)

When doing .broadcastMessage():

  this._uri: "app://system.gaiamobile.org/index.html"
  msg.uri: "app://system.gaiamobile.org/"

The page URIs don't match! It seems this issue only happens in System App.
(Assignee)

Comment 1

5 years ago
This should be a bb+ blocker which blocks Bug 797803.
Blocks: 755245, 797803
blocking-basecamp: --- → ?
(Assignee)

Comment 2

5 years ago
Created attachment 673178 [details]
Patch

Hi Fabrice,

This issue is very tricky. When starting up, the app will register all the messages based on the launch_path. When registering handlers, the app will register them based on the current page URI. Therefore, it'd be better to add the correct launch_path for System App so that it won't reject to handle the coming messages.

I think the previous change of matching page URIs is reasonable so I'd prefer keeping it and correcting the manifest part. Please let me know if you don't agree on this change.
Assignee: nobody → clian
Attachment #673178 - Flags: review?(fabrice)
Comment on attachment 673178 [details]
Patch

I agree that this is a gaia issue.
Attachment #673178 - Flags: review?(fabrice) → review+
Component: DOM → Gaia
Product: Core → Boot2Gecko
Version: Trunk → unspecified
(Assignee)

Comment 4

5 years ago
Need help to merge the pull request. Thanks!
Keywords: checkin-needed
Comment on attachment 673178 [details]
Patch

Manifest change only. a=me.
Attachment #673178 - Flags: approval-gaia-master+
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Keywords: checkin-needed

Updated

5 years ago
blocking-basecamp: ? → +
Keywords: regression
Priority: -- → P1

Updated

5 years ago
Whiteboard: QARegressExclude
You need to log in before you can comment on or make changes to this bug.