780837 - Investigate ways to show more recent downloads in the panel

Status: RESOLVED WORKSFORME

(Firefox :: Downloads Panel, defect)

Description

[:Paolo Amadini]

The maximum number of items shown in the Downloads Panel, instead of being
fixed and limited to a small number, could also be computed dynamically based
on the available space on the screen.

While the panel should never expand to fill all the available screen height, on
large screens it's possible that we want to show more items.

Comment 1

[:Paolo Amadini]

Some technical notes from bug 747903:

(Mano from comment #17)
> Paulo, please see SetPopupPosition and its helpers in in nsMenuPopupFrame.
> In short, all you need is a JS helper that mimics its available-area
> detection. Unless I missed something, it does not use any API that's not
> available to JS (it uses nsIScreenManager and nsIScreen).
> If you're looking for an example of using this api in js, see the dragend
> handler in tabbrowser.xml

(Mano from comment #19)
> For getting the available height, I think it should be as easy as:
> 
> window.screen.availHeight - element.boxObject.screenY
> 
> window.screen.availHeight, iirc, only considers the monitor on which the
> window is placed, so this should Just Work. So, you may not even need to go
> though the gfx interface.

Comment 2

[:Paolo Amadini]

(In reply to Paolo Amadini [:paolo] from comment #1)
> (Mano from comment #19)
> > For getting the available height, I think it should be as easy as:
> > 
> > window.screen.availHeight - element.boxObject.screenY

Since we should leave a lot of free space from the screen's vertical border, that
might work (the button's height itself is negligible), of course with the caveat
that we must consider the case the panel opens over the button.

Also, we might even want the number of items to be fixed regardless of the
button's position, thus always less than half the screen's height minus some
border space (so, probably even simpler).

> > window.screen.availHeight, iirc, only considers the monitor on which the
> > window is placed, so this should Just Work. So, you may not even need to go
> > though the gfx interface.

We should also understand whether the number of items should be variable
depending on the current window's screen (probably so), or the lowest of all
screens so that it can fit in any screen and the number of items doesn't change.

I have another technical question, however: can I compute the height of one
download item, based on the currently applied theme, while the panel is still
closed? I currently populate the list when the panel is invisible, if I cannot
get that value at that time, I might have to rethink the limitation logic.

Comment 3

[Asaf Romano (gone)]

Taking the button size into account is easy.

It's somewhat hard to compute the size of each item, but _at worst_ we can use an hardcoded size, maybe multiplied by the font size somehow. It's any worse than than hardcoding "3 items" ;)

Anyway, once we figure out we want to do this, I'll come up with something. I promise :)

Comment 4

[:Paolo Amadini]

(In reply to Paolo Amadini [:paolo] from comment #1)
> (Mano from comment #19)
> > For getting the available height, I think it should be as easy as:
> > 
> > window.screen.availHeight - element.boxObject.screenY

It was too simple to be true... in fact, this doesn't work :-(

The fact is window.screen.availHeight only considers the current screen (where
the majority of the window's rectangle resides), while boxObject.screenY is not
expressed in screen coordinates but uses the whole virtual viewport's coordinates.

We might need to do some more computations based on nsIScreenManager.screenForRect.
That's probably feasible, just needs a little bit more thought.

By the way, I don't see a way to enumerate all the available screens in
nsIScreenManager. If there's one, we might also consider to always use half of
the height of the shortest screen.

Comment 5

[:Paolo Amadini]

(In reply to Paolo Amadini [:paolo] from comment #1)
> (Mano from comment #17)
> > If you're looking for an example of using this api in js, see the dragend
> > handler in tabbrowser.xml

Yeah, maybe we could do something like this.

Comment 6

[:Paolo Amadini]

The main problem here is getting the height of one the download item. Testing
shown that calling getComputedStyle is unreliable while the download item is invisible, returning different values in different moments.

The only reliable way to get the actual item height, in device pixel units of the
screen where the panel is shown, seems to be to wait for the panel to be actually
displayed, and then access the box object of the download item.

To avoid flickering as we add new items to the panel, we could display the panel
with "visibility: hidden;", compute the maximum number of items that fit on
the screen, add more items to the list if necessary, and then switch back to
"visibility: visible;".

However, if I do 'this.panel.style.visibility = "hidden";' before openPopup, and
then 'this.panel.style.visibility = "visible";' in the onPopupShown handler, the
panel is visible only the first time it's shown. The other times, the visibility
style reads "visible" but the panel content area remains totally transparent.

Any ideas?

Comment 7

[Asaf Romano (gone)]

Paolo, could you attach a patch I could play with? Either you've uncovered a major layout bug, or there's a little mistake somewhere.

Comment 8

[:Paolo Amadini]

Attachment: Testing the "visibility" style

(In reply to Mano from comment #7)
> Paolo, could you attach a patch I could play with? Either you've uncovered a
> major layout bug, or there's a little mistake somewhere.

Just apply this patch, then open the panel two times. It will be visible only
the first time. The second time, in the Error Console you should see "Hide!
Hide! Show! Hide! Show!". Note that the first time the function re-enters so
you see "Hide!" two times, this however doesn't change the fact that the last
style we set on the panel is "visible".

Comment 9

[:Paolo Amadini]

(In reply to Paolo Amadini [:paolo] from comment #8)
> Note that the first time the function re-enters so
> you see "Hide!" two times, this however doesn't change the fact that the last
> style we set on the panel is "visible".

Note, in August I had also tested this with a temporary fix for the re-entrancy
issue, with the same result.

Comment 10

[Marco Bonardo [:mak]]

Note that this may not be a good solution yet, the problem is that any fixed amount of entries are going to be confusing when the button shows a download is in progress but that download has been cut off from the panel.  We need a better handling here.

Comment 12

[Florian Bender]

Proposal: Make the downloads list scrollable. When opening the panel with a simple height transition, set maxHeight to the smallest possible one-item size. Then (while the transition is playing) calculate the max panel height and adjust the max-height accordingly (thus the transition should continue seamlessly). A plus: Even if the screen is small, an arbitrary number of downloads can be shown without opening the downloads manager.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

(In reply to Paolo Amadini [:paolo] from comment #9)
> (In reply to Paolo Amadini [:paolo] from comment #8)
> > Note that the first time the function re-enters so
> > you see "Hide!" two times, this however doesn't change the fact that the last
> > style we set on the panel is "visible".
> 
> Note, in August I had also tested this with a temporary fix for the
> re-entrancy
> issue, with the same result.

The double-hide at the beginning seems to have been fixed - I suspect that the patch for bug 773267 fixed it.

I'm now investigating the approach suggested above - using visibility: hidden items to calculate the max number of items to display.

Comment 14

[Marco Bonardo [:mak]]

(In reply to Mike Conley (:mconley) from comment #13)
> I'm now investigating the approach suggested above - using visibility:
> hidden items to calculate the max number of items to display.

Remember that if the toolbar is on the bottom of the screen the arrow panels open up above the window, in this case whatever you calculate may end up being bogus.

Regardless how many entries we show, we need to better sum-up what's not visible as well.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Bonardo [:mak] from comment #14)
> (In reply to Mike Conley (:mconley) from comment #13)
> > I'm now investigating the approach suggested above - using visibility:
> > hidden items to calculate the max number of items to display.
> 
> Remember that if the toolbar is on the bottom of the screen the arrow panels
> open up above the window, in this case whatever you calculate may end up
> being bogus.
> 
> Regardless how many entries we show, we need to better sum-up what's not
> visible as well.

Good point.

For now, I'm simply starting by getting a sample of the richlistitem height the first time an item is added to the list - I assume that this sample is useful, regardless of where the toolbar is placed, and which way the arrow panel opens.

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Attachment: WIP Patch 1

Sampling the item height the first time the panel is displayed with 1 or more items in it.

Comment 17

[Mike Conley (:mconley) (:⚙️)]

Attachment: Tentative wireframe

So here's what I'm targeting until I get something from Stephen.

Not sure if the global download rate is correct - I'm mainly focusing on the final item in the list that displays the "invisible" completed and in-progress downloads.

Comment 18

[:Paolo Amadini]

I filed bug 808277 for the problem of making it clear that there is overflow,
that is in fact independent from how many items are actually displayed.

Comment 19

[Marco Bonardo [:mak]]

I'm making this not blocking the feature.
While would be nice to have more contents, there are so many edge cases on screen size and panel position, we may end up spending all of the time just working on them.
There are clearly higher priorities than bringing downloads from 3 to 5 or whatever else. Bug 808277 is what is really blocking us as of now.

Comment 23

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

This spec by shorlander should help:
http://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html

Comment 24

[Marco Bonardo [:mak]]

we increased tdhe number of downloads in th panel already and it's unclear what's the left scope here. What we did already was basically what we wanted to do. We could re-evaluate in the future, but for now keeping this open is not useful.