Closed Bug 1496706 Opened 6 years ago Closed 6 years ago

Crash in mozilla::widget::ToastNotificationHandler::OnActivate

Categories

(Toolkit Graveyard :: Notifications and Alerts, defect, P1)

Unspecified
Windows 8
defect

Tracking

(firefox-esr60 unaffected, firefox62 unaffected, firefox63 unaffected, firefox64 fixed, firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: calixte, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is report bp-4db7fbf2-bda2-4ab7-ab00-869370181005. ============================================================= Top 10 frames of crashing thread: 0 xul.dll HRESULT mozilla::widget::ToastNotificationHandler::OnActivate widget/windows/ToastNotificationHandler.cpp:402 1 xul.dll HRESULT Microsoft::WRL::Details::DelegateArgTraits<long vs2017_15.8.4/SDK/Include/10.0.17134.0/winrt/wrl/event.h:245 2 rpcrt4.dll Invoke 3 wpnapps.dll wpnapps.dll@0x425f 4 wpnapps.dll wpnapps.dll@0x4e00 5 rpcrt4.dll LRPC_BINDING_HANDLE::CleanupOrphanedCasaulFlowEntries 6 rpcrt4.dll NdrStubCall3 7 combase.dll local_unwind 8 combase.dll local_unwind 9 combase.dll CCtxComChnl::ContextInvoke ============================================================= There is 1 crash in nightly 64 with buildid 20181004224156. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1155505. [1] https://hg.mozilla.org/mozilla-central/rev?node=baf52bca95ae
Flags: needinfo?(m_kato)
inspectable seems to be nullptr
Assignee: nobody → m_kato
Flags: needinfo?(m_kato)
Blocks: 1497425
When clicking toast notification window, OnActivate is called. When using Windows 8, 2nd parameter (IInspectable) might be nullptr. So when this parameter is nullptr, it should recognize as alertclickcallback with mClickable=true.
Status: NEW → ASSIGNED
Priority: -- → P1
Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/39abbcc4230b Windows 8 doesn't return valid IInspectable when clicking toast notification. r=aklotz
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Please nominate this for Beta approval when you get a chance.
Flags: needinfo?(m_kato)
Comment on attachment 9015491 [details] Bug 1496706 - Windows 8 doesn't return valid IInspectable when clicking toast notification. r?aklotz [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1155505 User impact if declined: When setting alerts.useSystemBackend=true (default is false on beta and release), Firefox uses OS native Toast notification on Windows 8+. But when clicking notification on Windows 8, Firefox will crash. Is this code covered by automated tests?: No Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: - Step 1. Set up Windows 8 2. alerts.useSystemBackend=true by about:config 3. Browse https://web-push-book.gauntface.com/demos/notification-examples/ 4. Click [EXAMPLE NOTIFICATION] 5. click notification window List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): nullptr check only for parameter. Also, this feature is off on beta and release, so this doesn't occur on default setting. String changes made/needed: No
Flags: needinfo?(m_kato)
Attachment #9015491 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Comment on attachment 9015491 [details] Bug 1496706 - Windows 8 doesn't return valid IInspectable when clicking toast notification. r?aklotz Fixes a crash when the alerts.useSystemBackend is enabled. Approved for 64.0b4.
Attachment #9015491 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi, I can't seem to reproduce the issue at all by clicking on the notification toast. I tried to reproduce it in Nightly 64 with build id 20181004224156 under Windows 8 (x64) without luck. Another thing is that in the Nightly build in which I tried reproducing the issue , the pref. alerts.useSystemBackend is true by default, and by disabling it, there is no change in terms of notifications at all. Can you still reproduce it? and if so, can you please write down the exact steps to reproduce it? Thank you.
Flags: needinfo?(m_kato)
(In reply to Sasca Catalin, QA [:csasca] from comment #9) > Hi, I can't seem to reproduce the issue at all by clicking on the > notification toast. I tried to reproduce it in Nightly 64 with build id > 20181004224156 under Windows 8 (x64) without luck. Another thing is that in > the Nightly build in which I tried reproducing the issue , the pref. > alerts.useSystemBackend is true by default, and by disabling it, there is no > change in terms of notifications at all. Can you still reproduce it? and if > so, can you please write down the exact steps to reproduce it? Thank you. I think that you don't use *.exe installer. To enable native Toast Notification, you have to use *.exe installer instead of *.zip package.
Flags: needinfo?(m_kato)
I tried reproducing the issue again, but this time with the .exe's for (20181004224156) and (20181004100222), by clicking on the [Example Notification] and the other as well, without success. I believe the native toast notification is the one in the attachment I'll be uploading here, right? The same notification as in the attachment is present whether the pref. alerts.useSystemBackend is set to true or false. The OS I use is the same (Windows 8 x64).
Flags: needinfo?(m_kato)
(In reply to Sasca Catalin, QA [:csasca] from comment #11) > Created attachment 9020700 [details] > 45114749_336681053545276_6514687513620119552_n.png > > I tried reproducing the issue again, but this time with the .exe's for > (20181004224156) and (20181004100222), by clicking on the [Example > Notification] and the other as well, without success. I believe the native > toast notification is the one in the attachment I'll be uploading here, > right? The same notification as in the attachment is present whether the > pref. alerts.useSystemBackend is set to true or false. The OS I use is the > same (Windows 8 x64). Do you select creating shortcut in desktop when installer suggests shortcut? Also, I have verified this using Nightly 2018-10-28 (x64).
Flags: needinfo?(m_kato)
And this pref is false in beta. You have to set to true if using beta.
(In reply to Sasca Catalin, QA [:csasca] from comment #11) > Created attachment 9020700 [details] > 45114749_336681053545276_6514687513620119552_n.png > > I tried reproducing the issue again, but this time with the .exe's for > (20181004224156) and (20181004100222), by clicking on the [Example > Notification] and the other as well, without success. I believe the native > toast notification is the one in the attachment I'll be uploading here, > right? The same notification as in the attachment is present whether the > pref. alerts.useSystemBackend is set to true or false. The OS I use is the > same (Windows 8 x64). Agh, this is another issue. Beta cannot enable toast notification now... So you don't need verify this issue.
I haven't tried on Beta builds, they are Nightly builds. And in these nightly builds I tried reproducing the issue, the pref is set on true by default in these builds, with new profile created. I let the installer on the default settings, and it created a shortcut on desktop, but I cannot see the connection between the shortcut and notification.
Flags: needinfo?(m_kato)
(In reply to Sasca Catalin, QA [:csasca] from comment #15) > I haven't tried on Beta builds, they are Nightly builds. And in these > nightly builds I tried reproducing the issue, the pref is set on true by > default in these builds, with new profile created. I let the installer on > the default settings, and it created a shortcut on desktop, but I cannot see > the connection between the shortcut and notification. If using Nightly, it will be bug 1496650.
Flags: needinfo?(m_kato)
Removing qe-verify+ since I could not reproduce, if someone can properly reproduce please verify it on latest 64 beta and latest 65 nightly.
Flags: qe-verify+
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: