Closed Bug 1020630 Opened 6 years ago Closed 6 years ago

Desktop Notification doorhanger/notification bar is no longer shown

Categories

(SeaMonkey :: Passwords & Permissions, defect)

Product:
SeaMonkey
Component:
Passwords & Permissions
Version:
SeaMonkey 2.27 Branch
Type:
defect
Priority:
Not set

Tracking

(seamonkey2.26 unaffected, seamonkey2.27 wontfix, seamonkey2.28 fixed, seamonkey2.29 fixed)

Status:
RESOLVED FIXED
Milestone:
seamonkey2.29
Tracking Flags:
Tracking Status
seamonkey2.26 --- unaffected
seamonkey2.27 --- wontfix
seamonkey2.28 --- fixed
seamonkey2.29 --- fixed

People

(Reporter: rsx11m.pub, Assigned: neil)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

Proposed patch
2.41 KB, patch
philip.chee
: review+
rsx11m.pub
: feedback+
iann_bugzilla
: approval-comm-beta+
Details | Diff | Splinter Review 
+++ This bug was initially created as a clone of Bug #1019986 +++

(Quoting rsx11m from bug 1019986 comment #1)
> Like the patch in bug 998787, this works fine in a SM 2.25, but no
> notification shows up on current trunk builds, which likely is a different
> bug (and considering bug 1019583, it may indicate a more general problem).

The oldest build I have which exhibits the problem is 2.27a1 2014-02-14, built from comm-central changeset 1ce77c2f9bd0 (on Linux). SeaMonkey 2.26 release builds are not affected (Windows/Linux).

Note that the regression range is substantially different from that for the offline-apps issue which I reported in bug 1019583, so this appears to be an independent regression.
Blocks: 595437
This is not restricted to the doorhanger, the old notification bar is no longer shown either when browser.doorhanger.enabled is toggled to true. Thus, more likely a backend issue failing to trigger the event showing the notification than the doorhanger implementation itself.
Summary: Desktop Notification doorhanger is no longer shown → Desktop Notification doorhanger/notification bar is no longer shown
Blocks: 1020657
Looks like bug 853356 is a good candidate for having caused this regression (bug 998787 comment #14).
Attached patch Proposed patch — — Splinter Review 
Sorry about the long line, but I couldn't decide how to wrap it.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8434563 - Flags: review?(philip.chee)
Attachment #8434563 - Flags: review?(iann_bugzilla)
Attachment #8434563 - Flags: feedback?(rsx11m.pub)
Comment on attachment 8434563 [details] [diff] [review]
Proposed patch

WFM on trunk.
Attachment #8434563 - Flags: feedback?(rsx11m.pub) → feedback+
Comment on attachment 8434563 [details] [diff] [review]
Proposed patch

> Sorry about the long line, but I couldn't decide how to wrap it.

How about:
    const nsIContentPermissionType = Components.interfaces.nsIContentPermissionType;
    var type = aRequest.types.queryElementAt(0, nsIContentPermissionType).type;

>      const kFeatureKeys = { "geolocation" : "geo",
I think this could be turned into a Map.
Attachment #8434563 - Flags: review?(philip.chee) → review+
Pushed comm-central changeset 37140ce93570.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-seamonkey2.29: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.29
Attachment #8434563 - Flags: review?(iann_bugzilla)
Comment on attachment 8434563 [details] [diff] [review]
Proposed patch

[Triage Comment]
a=me for 2.28 which is now on c-b
Attachment #8434563 - Flags: approval-comm-beta+
Assuming that this is won't-fix for 2.27 as it's not going to be released anyway.
status-seamonkey2.27: affected → wontfix
You need to log in before you can comment on or make changes to this bug.