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

Status: VERIFIED FIXED

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

Description

[Ioana Crisan]

Attachment: 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.

Comment 1

[Fischer [:Fischer]]

(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.

Comment 2

[Ioana Crisan]

(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

Comment 3

[Fischer [:Fischer]]

Attachment: Bug 1377852 - Do not display the 2nd storage pressure notification when there is one being displayed,

Review commit: https://reviewboard.mozilla.org/r/169528/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169528/

Comment 4

[Mozilla Autoland]

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)

Comment 5

[Fischer [:Fischer]]

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/1-2/

Comment 6

[Mozilla Autoland]

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)

Comment 7

[Mozilla Autoland]

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)

Comment 8

[Fischer [:Fischer]]

Update the bug title to reflect what this bug is addressing.

Comment 9

[Fischer [:Fischer]]

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/

Comment 10

[Fischer [:Fischer]]

(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 11

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 12

[Fischer [:Fischer]]

(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.

Comment 13

[Mozilla Autoland]

hg error in cmd: hg update --clean -r 4a092af42641: abort: unknown revision '4a092af42641'!

Comment 14

[Fischer [:Fischer]]

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/3-4/

Comment 15

[Fischer [:Fischer]]

Rebased on the latest Central.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e19920c0fd8
Do not display the 2nd storage pressure notification when there is one being displayed, r=jaws

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8e19920c0fd8

Comment 18

[Paul Silaghi, QA [:pauly]]

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.