Closed
Bug 546502
Opened 15 years ago
Closed 15 years ago
Refreshing on toolbar for crash plugin UI doesn't remove notification bar
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 3.7a5
Tracking | Status | |
---|---|---|
status1.9.2 | --- | .4-fixed |
People
(Reporter: tchung, Assigned: Dolske)
References
()
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gavin
:
review+
christian
:
approval1.9.2.4+
|
Details | Diff | Splinter Review |
If you refresh a crashed plugin with the notification window open using the toolbar Refresh button, it will not hide the plugin UI crash notification bar.
Repro:
1) run nightly trunk:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a2pre) Gecko/20100216 Minefield/3.7a2pre
2) set ipc to true
3) open a flash plugin and force kill the process. I used URL: http://www.vh1.com/video/frank-the-entertainer-in-a-basement-affair/full-episodes/meet-the-rest-of-the-marescas/1631827/playlist.jhtml
4) Notice the notification crash UI drop down ("The Shockwave Flash plugin has crashed")
5) now click the "refresh" button on the toolbar menu.
6) Verify the page refreshes with the plugin reloading, but the notification bar doesn't hide.
However, clicking the "reload page" button on the notification will hide the notification bar.
* See screenshot
Expected:
- Refreshing via toolbar will hide the notification bar.
Comment 1•15 years ago
|
||
Looks like there needs to be some code similar to when you dismiss other notification bars like: installing plugin's or adding lwthemes (and don't have whitelisted getpersonas.com) where clicking on a button also removes the bar after it is no longer needed.
Comment 2•15 years ago
|
||
I can't see any notification toolbar in today's builds. It is possible that notification bar was removed. In this case, also this bug is fixed.
Something broke here. Open gmail and two youtube videos. Focus one of the youtube videos. Kill the m-r.exe. The notification bar does now show up on the other youtube video's tab but does show up on the gmail tab.
Seems like the notification bar is only showing for other sites.
(In reply to comment #3)
> Something broke here. Open gmail and two youtube videos. Focus one of the
> youtube videos. Kill the m-r.exe. The notification bar does now show up on
> the other youtube video's tab but does show up on the gmail tab.
>
> Seems like the notification bar is only showing for other sites.
Ignore this. I misunderstood when the notification bar is supposed to show and i think Maini did also.
"the notification bar on pages where the plugin is too small to see the plugin-crashed UI directly"
Maini, try on sites that has very small plugin content. Gmail is one such site.
Comment 5•15 years ago
|
||
It seems that Gmail page contains Flash plugin. When you kill mozilla-runtime the plugin-crashed UI appears in youtube video's tab and the notification bar in Gmail's tab.
If you click reload in plugin-crashed UI video restarts, but plugin in Gmail's page isn't reloaded. Otherwise, if you click notification bar in Gmail's page plugin restarts, but youtube video's tab continues to show crashed UI.
This seems like a correct behaviour in my opinion.
Assignee | ||
Comment 6•15 years ago
|
||
The specific problem here is that the notification bar was only being removed when clicking its "reload page" button. Other methods of reloading the page don't trigger the button's callback, and since the location doesn't change the notification bar doesn't remove itself (as would happen if clicking a link in the page).
Adding an unload handler seems the easiest way to notice when things were reloaded. [I do this on window.top, since it would be complicated to track a tab full of frames, and the UX would be confusing.]
Assignee: nobody → dolske
Attachment #427199 -
Attachment is obsolete: true
Attachment #438024 -
Flags: review?(gavin.sharp)
Comment 7•15 years ago
|
||
Comment on attachment 438024 [details] [diff] [review]
Patch v.1
>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
>+ // Remove the notfication when the page is reloaded.
>+ doc.defaultView.top.addEventListener("unload", function() {
>+ notificationBox.removeNotification(notification); }, false);
I prefer:
doc.defaultView.top.addEventListener("unload", function() {
notificationBox.removeNotification(notification);
}, false);
Attachment #438024 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 8•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Assignee | ||
Updated•15 years ago
|
Attachment #438024 -
Flags: approval1.9.2.4?
Comment 9•15 years ago
|
||
The unload handler is going to break the bfcache, AFAIK; pagehide should be used instead.
Updated•15 years ago
|
Component: Plug-ins → General
Product: Core → Firefox
QA Contact: plugins → general
Target Milestone: mozilla1.9.3a5 → ---
Updated•15 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Assignee | ||
Comment 10•15 years ago
|
||
(In reply to comment #9)
> The unload handler is going to break the bfcache
That actually seems desirable; the handler is only added when the page has a (small) crashed plugin, and I don't see what we'd want to cache that state.
Comment 11•15 years ago
|
||
Shouldn't that be handled at a lower level? How was this being dealt with without this patch? It sounds like the wrong fix to a different problem.
Comment 12•14 years ago
|
||
It wasn't being dealt with without this patch. It's an unintended side effect of this patch that turned out to be beneficial. It could also be handled at a lower level, but I don't think we really need to worry about it. Might be worth adding a comment, though.
Comment 13•14 years ago
|
||
Comment on attachment 438024 [details] [diff] [review]
Patch v.1
a=LegNeato for 1.9.2.4. Please land on both mozilla-1.9.2 default and
GECKO1924_20100413_RELBRANCH
Attachment #438024 -
Flags: approval1.9.2.4? → approval1.9.2.4+
Assignee | ||
Comment 14•14 years ago
|
||
Pushed to 1.9.2...
relbranch: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2de16fe1b3af
default: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/08f05330eb8b
status1.9.2:
--- → .4-fixed
Target Milestone: --- → Firefox 3.7a5
Comment 15•14 years ago
|
||
I'm not sure how to verify this since we don't use a notification bar anymore for OOPsies. Tony?
Comment 16•14 years ago
|
||
We still show a notification bar for plugin crashes, but only if the plugin object is too small to show the in-page UI.
You need to log in
before you can comment on or make changes to this bug.
Description
•