PopupNotification is not attached to anchor

NEW
Unassigned

Status

()

Toolkit
Notifications and Alerts
7 years ago
4 years ago

People

(Reporter: MyKey_, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [doorhanger])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Build Identifier: Firefox 4.0b11

PopupNotifications.show() does not attach the notification to the supplied anchorId. 

Reproducible: Always

Steps to Reproduce:
1. Execute the JavaScript command to show a notification attached to the addon bar on the bottom:
PopupNotifications.show(gBrowser.selectedBrowser,
  "id",
  "message",
  "addon-bar", /* <- anchorID */
  null, /* mainAction */
  null, /* secondaryActions */
  null /* options */
);

Actual Results:  
The notification is always attached to the original "notification-popup-box".

Expected Results:  
The notification should be attached to the addon bar ("addon-bar") on the bottom.

In PopupNotifications.jsm in line 76, the querySelector function returns null even if the element with the supplied anchorID exists (e.g., "addon-bar", "TabsToolbar", or even "notification-popup-box").

[code]
    anchorElement = this.owner.iconBox.querySelector("#"+this.anchorID);

    if (!anchorElement)
      anchorElement = this.owner.iconBox;
[/code]

Updated

7 years ago
Whiteboard: [doorhanger]
Can you please create a test case and attach it to this bug so that we can try to reproduce it?

Thanks.
(Reporter)

Comment 2

7 years ago
(In reply to comment #1)
> Can you please create a test case and attach it to this bug so that we can try
> to reproduce it?
> 
> Thanks.

You can reproduce this error by following these steps (in this exact order): 

1. Open Firefox 4.0b11
2. Navigate to "about:addons"
3. Open the Web Console (e.g. through Control-Shift-K)
4. Execute the following lines of code in the Web Console command line interpreter:

testMsgPos = function(anchorId) {
	
	// get main window:
	var w = QueryInterface(Components.interfaces.nsIInterfaceRequestor)
					   .getInterface(Components.interfaces.nsIWebNavigation)
					   .QueryInterface(Components.interfaces.nsIDocShellTreeItem)
					   .rootTreeItem
					   .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
					   .getInterface(Components.interfaces.nsIDOMWindow);
	
	var nfId = anchorId + "_notification_id";
	var nfMsg = "This message should be attached to \""  + anchorId + "\".";
	
	// display notification attached to anchor:
	var nf = w.PopupNotifications.show(w.gBrowser.selectedBrowser,
	  nfId,
	  nfMsg,
	  anchorId, /* <- anchorId */
	  null, /* mainAction */
	  null, /* secondaryActions */
	  null /* options */
	);
	
	// get anchor element:
	var anchor = nf.anchorElement;
	
	// report errors to firefox error console:
	if(!anchor) console.error( "Notification has no anchor" ); // report the error and continue execution
	else if(anchor.id != anchorId) console.error( "Found anchor.id '" + anchor.id + "' but expected anchorId '" + anchorId + "'!" ); // report the error and continue execution

	// return whether anchor is present and has the expected id:
	return anchor && anchor.id == anchorId;
};

testMsgPos("addon-bar");
testMsgPos("TabsToolbar");
testMsgPos("home-button");



5. You would expect that the 3 notifications would appear in different positions ("addon-bar", "TabsToolbar", and "home-button"). But they don't.
6. The following error messages will be shown in the Web Console:
> [19:50:31.973] "Found anchor.id 'notification-popup-box' but expected anchorId 'addon-bar'!"
> [19:50:31.989] "Found anchor.id 'notification-popup-box' but expected anchorId 'TabsToolbar'!"
> [19:50:32.006] "Found anchor.id 'notification-popup-box' but expected anchorId 'home-button'!"
(Reporter)

Comment 3

7 years ago
Note that the above JavaScript code has two line breaks within the comments, which break the code. 

Before execution, please remove the two comments with the following text:

  //report the
error and continue execution

Sorry for the confusion.
Confirmed using the testcase from comment 2 in Firefox 4.0rc1.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

7 years ago
Here's the code from PopupNotifications.jsm

let anchorElement = null;
if (this.anchorID)
  anchorElement = this.owner.iconBox.querySelector("#"+this.anchorID);
 
if (!anchorElement)
  anchorElement = this.owner.iconBox;

So because querySelector is called on the iconBox, anchorID only works if the element specified by the anchorID is a child of the iconBox.

should be:

anchorElement = this.owner.iconBox.ownerDocument.querySelector("#"+this.anchorID);

Comment 6

7 years ago
When this will be fixed?
Can i do like that to fix it?

Components.utils.import("chrome://content/PopupNotifications.jsm");
PopupNotifications.show(gBrowser.selectedBrowser, "sample-popup",
        "This is a sample popup notification.",
        "MY-ANCHER-ID", /* anchor ID */
        {
          label: "Do Something",
          accessKey: "D",
          callback: function() {
            alert("Doing something awesome!");
          }
        },
        null  /* secondary action */
        );	

where chrome://content/PopupNotifications.jsm - is file with fixed code?

Comment 7

7 years ago
Created attachment 538325 [details] [diff] [review]
Use iconBox ownerDocument for query

Because querySelector is called on the iconBox, anchorID only works if the element specified by the anchorID is a child of the iconBox.

querySelector should be called on the ownerDocument of the iconBox
Attachment #538325 - Flags: review?

Updated

7 years ago
Attachment #538325 - Flags: review? → review?(gavin.sharp)

Comment 8

7 years ago
In comment above - chrome://content/PopupNotifications.jsm has 

anchorElement = this.owner.iconBox.ownerDocument.querySelector("#"+this.anchorID);

but it still not works - attached to iconBox, did i misunderstanding something?
Comment on attachment 538325 [details] [diff] [review]
Use iconBox ownerDocument for query

This was intentional - the _onIconBoxCommand code won't work for elements outside of the iconBox.

We need a better solution to allow PopupNotifications anchored elsewhere, but I haven't had much time to come up with one. Bug 604473 takes the approach of just creating a separate PopupNotifications object, which may be a suitable workaround in the interim.
Attachment #538325 - Flags: review?(gavin.sharp) → review-
The problem is people are reading the documentation:

https://developer.mozilla.org/en/JavaScript_code_modules/PopupNotifications.jsm

And it indicates that the popup can be attached arbitrarily.

If this:

anchorID
The ID of the element that should serve as the anchor for the notification popup (that is, the element the arrow on the popup should point to). If you specify null, the notification will be anchored to the icon box at the start of the URL bar.

Can only ever be the iconBox, we need to change the docs.
It can any element, it just needs to be a descendant of the iconBox.
Bulk move to Toolkit::Notifications and Alerts

Filter on notifications-and-alerts-component.
Component: General → Notifications and Alerts
You need to log in before you can comment on or make changes to this bug.