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)
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)
Bug 1496706 - Windows 8 doesn't return valid IInspectable when clicking toast notification. r?aklotz
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
(deleted),
image/png
|
Details |
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)
Assignee | ||
Comment 1•6 years ago
|
||
inspectable seems to be nullptr
Assignee: nobody → m_kato
Flags: needinfo?(m_kato)
Assignee | ||
Comment 2•6 years ago
|
||
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.
Updated•6 years ago
|
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
Comment 4•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Comment 5•6 years ago
|
||
Please nominate this for Beta approval when you get a chance.
Flags: needinfo?(m_kato)
Assignee | ||
Comment 6•6 years ago
|
||
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?
Updated•6 years ago
|
Flags: qe-verify+
Comment 7•6 years ago
|
||
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+
Comment 8•6 years ago
|
||
bugherder uplift |
Comment 9•6 years ago
|
||
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)
Assignee | ||
Comment 10•6 years ago
|
||
(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)
Comment 11•6 years ago
|
||
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)
Assignee | ||
Comment 12•6 years ago
|
||
(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)
Assignee | ||
Comment 13•6 years ago
|
||
And this pref is false in beta. You have to set to true if using beta.
Assignee | ||
Comment 14•6 years ago
|
||
(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.
Comment 15•6 years ago
|
||
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)
Assignee | ||
Comment 16•6 years ago
|
||
(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)
Comment 17•6 years ago
|
||
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+
Updated•1 year ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•