Closed Bug 1300556 Opened 3 years ago Closed 3 years ago

Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden

Categories

(Firefox :: Downloads Panel, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 51
Iteration:
51.3 - Sep 19
Tracking Status
firefox50 + verified
firefox51 + verified

People

(Reporter: magicp.jp, Assigned: Paolo)

References

Details

(Keywords: regression, Whiteboard: [fxprivacy])

Attachments

(2 files)

Attached video steps-to-reproduce.mp4
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160905030222

Steps to reproduce:

1. Start Nightly
2. Go to "https://www.mozilla.org/en-US/firefox/channel/"
3. Download "Firefox Beta"
4. Check downloads panel after downloading
5. Go to Home (about:home)
6. Click Downloads of launcher
7. Click "Clear Downloads"
8. Check downloads panel again


Actual results:

Spacer height is not reset even if clearing download history.

Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=4b767c8f023685e9e598f026453fbd906e0e8d1f&tochange=b655b3fb6153570e6e91635322e1e3c8ed2e4a1a


Expected results:

Spacer height is reset when clearing download history.
Blocks: 1280709
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Downloads Panel
OS: Unspecified → All
Hardware: Unspecified → All
Blocks: 1292345
[Tracking Requested - why for this release]:
Similar to bug 1292345. Following the discussion in that bug, I think we want this in Firefox 51 but a fix for Firefox 50 might be optional if it isn't simple to implement.
Keywords: regression
Whiteboard: [fxprivacy][triage]
Tracking 51+ for this downloads panel regression.
Assignee: nobody → paolo.mozmail
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Looks like this is getting ready to land.
Comment on attachment 8788900 [details]
Bug 1300556 - Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden.

https://reviewboard.mozilla.org/r/77220/#review75556

One question below but the other changes are fine. Would be nice if a test was included (add a download item to the panel, open the panel, note the height, close the panel, open the library, clear downloads, open the panel, confirm that the height is now less than the previous height).

::: browser/components/downloads/content/downloads.js
(Diff revision 1)
> -    DownloadsBlockedSubview.view.setHeightToFit();
> -
>      // If we've got some hidden downloads, we should activate the

Shouldn't we keep this in case the panel is opened and another download is started?
Attachment #8788900 - Flags: review?(jaws) → review+
Depends on: 1301413
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Would be nice if a test was included

I've filed a separate bug for the automated test case since I'm wary of potential intermittent failures after uplift, and I'll not be around next week. I plan to work on the test as soon as I'm back, since it will be useful even for the work I'm doing to refactor this code.

> Shouldn't we keep this in case the panel is opened and another download is
> started?

I've tested with a delayed download and expanding the panel while it's visible works correctly, it's just shrinking that is an issue. I removed this call to avoid flickering on open in the most common case, where downloads are just added to the panel while it's closed.
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/fx-team/rev/8f68c33f5793
Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden. r=jaws
https://hg.mozilla.org/mozilla-central/rev/8f68c33f5793
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Flags: qe-verify? → qe-verify+
Comment on attachment 8788900 [details]
Bug 1300556 - Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden.

Approval Request Comment
[Feature/regressing bug #]: Bug 1252509
[User impact if declined]: Similarly to bug 1292345, the height of the downloads panel will remain unnecessarily large, but in this case this happens if downloads are removed while the panel is hidden.
[Describe test coverage new/current, TreeHerder]: Just landed on mozilla-central, needs manual testing
[Risks and why]: Changes are limited to the height calculation for the Downloads Panel, the risk is only that we missed some cases and the panel will be too high, but still usable.
[String/UUID change made/needed]: None
Attachment #8788900 - Flags: approval-mozilla-aurora?
Summary: Don't increase spacer in download panel after downloading → Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden
Iteration: --- → 51.3 - Sep 12
verified in latest Nightly (20160909030428)
QA Contact: paul.silaghi
Status: RESOLVED → VERIFIED
Note that there is a visible auto-sizing of the downloads panel in this scenario - http://screencast.com/t/ZH5k7D5kc
Can we do something here?
NI Jared since Paolo is away.
Flags: needinfo?(jaws)
I would consider this a known issue that will hopefully get fixed by bug 1009116.
Flags: needinfo?(jaws)
>Status: RESOLVED → VERIFIED

Comment 9, How about uplift?
Flags: needinfo?(paul.silaghi)
(In reply to magicp from comment #13)
> Comment 9, How about uplift?
Hi Ritu, perhaps can you answer this?
Flags: needinfo?(paul.silaghi) → needinfo?(rkothari)
Comment on attachment 8788900 [details]
Bug 1300556 - Adjust the height of the Downloads Panel if downloads are removed while the panel is hidden.

It's a minor UI issue, but since the fix was verified on Nightly and 50 is still in aurora cycle, let's take it.
Flags: needinfo?(rkothari)
Attachment #8788900 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
verified in 50.0a2 (Build ID 20160914004005)
You need to log in before you can comment on or make changes to this bug.