Closed Bug 1377852 Opened 7 years ago Closed 7 years ago

Do not display the 2nd storage pressure notification when there is one being displayed

Categories

(Core :: DOM: Push Subscriptions, defect, P2)

Product:
Core
Component:
DOM: Push Subscriptions
Version:
56 Branch
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- verified

People

(Reporter: icrisan, Assigned: Fischer)

References

(Blocks 1 open bug)

Details

(Whiteboard: [storage-v1])

Attachments

(2 files)

NotificationStack.mp4
332.22 KB, video/mp4
Details
Bug 1377852 - Do not display the 2nd storage pressure notification when there is one being displayed,
59 bytes, text/x-review-board-request
jaws
: review+
Details
Attached video NotificationStack.mp4 — 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170701181359

Steps to reproduce:
1. Launch Nightly 56.0a1
2. Set the "browser.storageManager.pressureNotification.minIntervalMS" pref to "1000"(1s)
3. Open an url(e.g: https://bug1377104.bmoattachments.org/attachment.cgi?id=8882153) 
4. Persist the site from step 3
5. Store data so the global limit is reached
6. Store more data 


Expected results:
Full disk warnings are not overlapped. Only one warning is displayed.

Actual results:
Full disk warnings are overlapped. Multiple warnings are displayed, one warning for each time the user tries to store more data from the global limit breach. Please see the attached video.
(In reply to Ioana Crisan from comment #0)
> Created attachment 8882995 [details]
> NotificationStack.mp4
> 
> User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101
> Firefox/56.0
> Build ID: 20170701181359
> 
> Steps to reproduce:
> 1. Launch Nightly 56.0a1
> 2. Set the "browser.storageManager.pressureNotification.minIntervalMS" pref
> to "1000"(1s)
> 3. Open an url(e.g:
> https://bug1377104.bmoattachments.org/attachment.cgi?id=8882153) 
> 4. Persist the site from step 3
> 5. Store data so the global limit is reached
> 6. Store more data 
> 
> 
> Expected results:
> Full disk warnings are not overlapped. Only one warning is displayed.
> 
> Actual results:
> Full disk warnings are overlapped. Multiple warnings are displayed, one
> warning for each time the user tries to store more data from the global
> limit breach. Please see the attached video.
Would you please upload the video again? Got the error message of "Couldn't play because the video is corrupt", thanks.
Flags: needinfo?(icrisan)
(In reply to Fischer [:Fischer] from comment #1)
> Would you please upload the video again? Got the error message of "Couldn't
> play because the video is corrupt", thanks.

Please try this link: https://drive.google.com/open?id=0B_L6mNdlzO5TOGV4ZU80SkItRFE
Flags: needinfo?(icrisan)
Assignee: nobody → fliu
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [storage-v1][triage] → [storage-v1]
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
Update the bug title to reflect what this bug is addressing.
Summary: Full disk warnings are overlapped → Do not display the 2nd storage pressure notification when there is one being displayed
(In reply to Fischer [:Fischer] from comment #9)
> Comment on attachment 8898162 [details]
> Bug 1377852 - Do not display the 2nd storage pressure notification when
> there is one being displayed,
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/169528/diff/2-3/
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9d611604f047da43286ab8429a578929e60ffa7
Comment on attachment 8898162 [details]
Bug 1377852 - Do not display the 2nd storage pressure notification when there is one being displayed,

https://reviewboard.mozilla.org/r/169528/#review175472

r=me, but if these should be one per host, then please change the notification value to include the host in it like the example I linked to below.

::: browser/base/content/browser.js:487
(Diff revision 3)
> +    if (notificationBox.getNotificationWithValue(NOTIFICATION_VALUE)) {
> +      // Do not display the 2nd notification when there is already one

Are these per domain or URI? Should we do like for feeds and still show multiple if they are for different domains or URIs?

http://searchfox.org/mozilla-central/source/browser/components/feeds/WebContentConverter.js#564,569
Attachment #8898162 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Comment on attachment 8898162 [details]
> Bug 1377852 - Do not display the 2nd storage pressure notification when
> there is one being displayed,
> 
> https://reviewboard.mozilla.org/r/169528/#review175472
> 
> r=me, but if these should be one per host, then please change the
> notification value to include the host in it like the example I linked to
> below.
> 
> ::: browser/base/content/browser.js:487
> (Diff revision 3)
> > +    if (notificationBox.getNotificationWithValue(NOTIFICATION_VALUE)) {
> > +      // Do not display the 2nd notification when there is already one
> 
> Are these per domain or URI? Should we do like for feeds and still show
> multiple if they are for different domains or URIs?
> 
> http://searchfox.org/mozilla-central/source/browser/components/feeds/WebContentConverter.js#564,569
Thank you for reminding this. No these are not per domain/URI. It warns when the total storage usage of all websites hits the limit.
Keywords: checkin-needed
hg error in cmd: hg update --clean -r 4a092af42641: abort: unknown revision '4a092af42641'!
Rebased on the latest Central.
Keywords: checkin-needed
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e19920c0fd8
Do not display the 2nd storage pressure notification when there is one being displayed, r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8e19920c0fd8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Reproduced the issue on Nightly 2017-06-12 using the testpage: https://bug1376444.bmoattachments.org/attachment.cgi?id=8881384
Verified fixed on 57.0a1 (2017-08-25) Win 7, Win 10, Ubuntu 14.04, OS X 10.12.6.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: