Closed Bug 1140275 Opened 5 years ago Closed 4 years ago
System Message API: message is sent to an app page even if not registered in manifest
Steps to reproduce: 1. Edit gaia/apps/clock/manifest.webapp and remove the "messages" section 2. Re-install gaia (make reset-gaia) 3. Open clock app and schedule a short alarm Expected result: Alarm does not alarm Actual result: Alarm alarms at the specified time However if we kill the clock app after step 3, the alarm does not alarm. See also bug 1137722 comment 10
Gaia patch - Allow required system messages for mochitest.
Assignee: nobody → selin
Attachment #8583637 - Flags: review?(fabrice)
Patch Gecko v1: Only send messages to pages registered in manifests.
Sean, can you explain the changes?
According to bug 1137722 comment 10, when we strictly prevent system messages from sending to app pages not registered in manifests, it would cause test failures on B2G emulators because the page URLs of those tests are not put in the manifest of the mochitest container app. So the Gaia patch is to whitelist those test pages. However, the URLs of those test pages may end with query strings like ?autorun=1&timeout=300&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flog%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&dumpOutputDirectory=%2Ftmp&manifestFile=tests.json Thus the patch also trims the tailing query string in the page URL registered for system messages. And it looks compatible with the actual usage, such as /dialer/index.html#keyboard-view, in Gaia apps.
I don't agree with doing that. It's perfectly valid to have different query strings for different messages with the same path. We need a better fix. Also make sure that this addresses the issue from comment #0 which looks slightly different to me.
Attachment #8583637 - Flags: review?(fabrice) → review+
Prevent system messages from being sent to app pages not registered in manifests. And fix relevant tests. (In comment 0, the generated system message to fire alarms may still be sent to the alarm page even when the page URL hasn't been listed in the app manifest.)
Fixing two more requestsync tests.
Comment on attachment 8642276 [details] [diff] [review] Patch, v3 Review of attachment 8642276 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8642276 - Flags: review?(fabrice) → review+
You need to log in before you can comment on or make changes to this bug.