Closed
Bug 747649
Opened 13 years ago
Closed 11 years ago
click to play plugin notification actions shouldn't explicitly remove the notification
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: Gavin, Unassigned)
References
Details
PopupNotification actions don't need to remove their notification, the PopupNotifications code handles that automatically.
There are a couple places where this happens:
- in the "never allow" action: http://hg.mozilla.org/mozilla-central/annotate/d1ac8e24872c/browser/base/content/browser.js#l7323
- in activatePlugins: http://hg.mozilla.org/mozilla-central/annotate/d1ac8e24872c/browser/base/content/browser.js#l7185
The latter can also be called from the click event handler - presumably it needs to keep removing the notification in that case, but not when called from a PopupNotification action.
Comment 1•13 years ago
|
||
Does it cause any problems if they do it? Those two were left on purpose. In activatePlugins as you noticed it's necessary for the case where it's called from the click handler, and it didn't seem worth to differentiate the two callers. The one in the "never allow" action we were going to get rid of it (last chunk in bug 711618 comment 29) but IIRC Jared said that would make a test easier, and it seemed harmless so we left it there.
Reporter | ||
Comment 2•13 years ago
|
||
(In reply to Felipe Gomes (:felipe) from comment #1)
> Does it cause any problems if they do it?
It looks like it's probably harmless.
> Jared said that would make a test easier, and it seemed harmless so we left it
> there.
I'd be interested in knowing how it can help tests, it seems pretty much equivalent.
Comment 3•13 years ago
|
||
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)
> I'd be interested in knowing how it can help tests, it seems pretty much
> equivalent.
For some reason the notification doesn't disappear while running the tests without this code. I think it's because of the way that I'm calling the callback in the test:
> popupNotification.secondaryActions[1].callback();
Is there a better way to call a secondary action of a notification? This method is probably skipping some pre/post-callback code paths.
Comment 4•13 years ago
|
||
I can fix the test so that it actually opens the doorhanger and runs the same code path that the user will see.
Assignee: nobody → jwein
Status: NEW → ASSIGNED
Updated•12 years ago
|
Comment 5•11 years ago
|
||
The notificiation no longer dismisses on button click by design.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•