607860 - remove navigator.mozNotification from Firefox desktop

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Doug Turner (:dougt)]

Jonas mentioned over IRC we should consider removing navigator.mozNotification from FF desktop.

I am not entirely convinced we should, but we should discuss.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We (and every other browser vendor) have been pushing hard the message that people should use feature detection rather than browser detection. So we want them to do things like

function myNotify(body, title, callback) {
  if ("Notification" in window) {
    var x = new Notification(myiconurl, title, body);
    x.onclick = callback;
  }
  else if ("mozNotification" in navigator) {
    var x = navigator.mozNotification.createNotification(myiconurl, title, body);
    x.show(callback);
  }
  else {
    ...fallback code, maybe use alert()...
  }
}

That doesn't work if we expose function which doesn't "work" on some platforms.

Comment 2

[Doug Turner (:dougt)]

i can add a pref that can be set in fennec (or by an addon for ff desktop).  sounds good, jonas?

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yup, that'd be awesome.

Comment 4

[Doug Turner (:dougt)]

Attachment: patch v.1

worked locally.  pushed to try.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 486681 [details] [diff] [review]
patch v.1

Looks good to me, but I'd like to have either peterv or jst look at the nsDOMClassInfo.cpp changes.

Comment 6

[Doug Turner (:dougt)]

peterv,  do you have time to review?

Comment 7

[Doug Turner (:dougt)]

jst/jonas.  i think we want this for ff4.

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 486681 [details] [diff] [review]
patch v.1

wanted for ff4.

Comment 9

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/6566ead94c4e

Comment 10

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/845b9487ffb3  (disabling tests)

Tests were disabled because they were failing:

5268 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/notification/test_basic_notification.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - Permission denied for <http://mochi.test:8888> to get property XPCComponents.classes at http://mochi.test:8888/tests/dom/tests/mochitest/notification/notification_common.js:76
5271 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/notification/test_basic_notification_click.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - Permission denied for <http://mochi.test:8888> to get property XPCComponents.classes at http://mochi.test:8888/tests/dom/tests/mochitest/notification/notification_common.js:76
5274 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/notification/test_leak_windowClose.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - is_feature_enabled is not defined at http://mochi.test:8888/tests/dom/tests/mochitest/notification/test_leak_windowClose.html:13

Because these tests do not test anything in Firefox (as we disabled this feature), we just removed notifications from the makefile dirs.

The feature is only enabled on mobile which uses IPC, and IPC mochitests do not completely work :(


When we re-enable for firefox desktop, we will re-enable these tests.