Closed Bug 1282662 Opened 8 years ago Closed 7 years ago

Redesign the download status information for each hover-item in downloads panel

Categories

(Firefox :: Downloads Panel, defect, P2)

Product:
Firefox
Component:
Downloads Panel
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
relnote-firefox --- 54+
firefox54 --- verified

People

(Reporter: selee, Assigned: selee)

References

Details

(Whiteboard: [CHE-MVP])

Attachments

(3 files)

Hover for More.pdf
251.57 KB, application/pdf
Details
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;
58 bytes, text/x-review-board-request
Paolo
: review+
Details
Bug 1282662 - Part 2: Implement the detail description in hover button case.;
59 bytes, text/x-review-board-request
Paolo
: review+
Details 
We need to consider the 9 kinds of state when an item is in hover case. This is based on UX spec https://mozilla.invisionapp.com/share/4Y6ZZH1E8#/screens/162255352 .

9 kinds of state
* Starting/Downloading
* Completed
* Cancelled
* Paused
* Deleted
* Failed
* Malware
* Uncommon Download
Whiteboard: [CHE-MVP]
Priority: -- → P2
Based on UX spec, Bryant helps to come out the major changes for the new design in the hover case:

* Starting/Downloading
  - Thread: Open after download
  - Button: cancel download
* Completed
  - Thread: Open file
  - Button: show in finder
* Cancelled
  - Thread: status/action time/source 
  - Button: restart download
* Paused
  - Thread: status/action time/source 
  - Button: resume download
* Deleted
  - Thread: status/action time/source 
  - Button: resume download
* Failed
  - Thread: status/action time/source 
  - Button: resume download
* Malware
  - Thread: show more information 
  - Button: show more information 
* Uncommon Download
  - Thread: show more information 
  - Button: show more information 
* Unwanted Download
  - Thread: show more information 
  - Button: show more information
Blocks: 1307064
Hi Paolo,

Could you take a look the spec [1] for changing the description of normal case (left) and hover case (right)? All these changes are relative to the string only. Thank you.

[1] https://mozilla.invisionapp.com/share/4Y6ZZH1E8#/screens/162255352
Flags: needinfo?(paolo.mozmail)
By looking at the specification page, it seems to me that the most important change is bug 812894, that I suggest doing first even if it's not related to changing the displayed string on hover.

We'll probably have time to talk about the other cases in more detail when we meet!
Flags: needinfo?(paolo.mozmail)
This bug will cover both normal and hover case implementation.
Hey Paolo,

I understand that the time string modification can be split to bug 812894. May I know why you think bug 812894 should be done first?
Except the shorten time string, do you think there are any other issues in this spec?

Thanks.
Flags: needinfo?(paolo.mozmail)
(In reply to Sean Lee [:seanlee][:weilonge] from comment #7)
> I understand that the time string modification can be split to bug 812894.
> May I know why you think bug 812894 should be done first?

Among all the changes, it's the one that users have actually been asking for.

> Except the shorten time string, do you think there are any other issues in
> this spec?

The main thing to keep into account when looking at a specification page like this is that the scope of the work we are expected to do as engineers cannot be clearly defined from it alone.

Depending on how you read it, a simple string like "open when done" might imply that we need an entire new functionality, or maybe we should simply ignore it and do as if it wasn't in the page. (For the record, the answer here is that this functionality is out of scope.)

When engineers do the breakdown, they should clarify which aspects should, and can, be implemented as an individual chunk of work. As you know, this is often an ongoing discussion between the engineering and the user experience teams. Marking bug 1269964 as a duplicate of this one was the right thing to do, because both are related, but still this bug is very broad in scope so it's unclear what we'll end up implementing.

Since I lack most of the context behind the specification, I believe that if I just tried to interpret it from my own point of view, this might just become an unfruitful exercise in written communication. It's probably best that we discuss in person, since we have this opportunity.

The notable exception is bug 812894 which is clearly defined in scope, and has already been requested by users.
Flags: needinfo?(paolo.mozmail)
(In reply to :Paolo Amadini from comment #8)
> Among all the changes, it's the one that users have actually been asking for.

But there's no actual dependency that this bug (bug 1282662) must be done after bug 812894, right?

> Since I lack most of the context behind the specification, I believe that if
> I just tried to interpret it from my own point of view, this might just
> become an unfruitful exercise in written communication. It's probably best
> that we discuss in person, since we have this opportunity.

How about a Vidyo call with Taipei UX designer to clarify the context behind spec?

Instead of waiting 10 days and leaving all remaining work to Hawaii, there's still work engineers can do including understanding reviewer's concern in advance, preparing patches, and leave only the key part - patch review - to Hawaii. Vidyo discussion in advance may not be the most efficient communication as F2F one, but definitely helps us a more efficient Hawaii meet on project deliverable.

Feel free to let us know for any difficulty. Thanks.
Flags: needinfo?(paolo.mozmail)
Attached file Hover for More.pdf — 
I've added more notes and attached the spec for "hover for more", hope this can provide more context. If can't, let's discuss based on the spec then. Thanks.
Assign to Sean to help clarify and implement.
Assignee: nobody → selee
(In reply to Ben Tian [:btian] from comment #9)
> But there's no actual dependency that this bug (bug 1282662) must be done
> after bug 812894, right?

The specification page attached to this bug shows a new time format and the download speed.

If you consider that as a required part of completing this bug, then there is a dependency. If you don't, then you can consider this bug complete without that change.

When Sean asked me to "take a look at the spec" in comment 3, I thought I was requested to give my input on which changes I saw as more important in the specification.

If this isn't what you asked, then what kind of information did you want me to provide with comment 3? I may have misunderstood what I was being asked, sorry for that.

> How about a Vidyo call with Taipei UX designer to clarify the context behind
> spec?

It's a good idea, but I'm intermittently available this week so it won't be possible to schedule one.

> Instead of waiting 10 days and leaving all remaining work to Hawaii, there's
> still work engineers can do including understanding reviewer's concern in
> advance, preparing patches, and leave only the key part - patch review - to
> Hawaii.

I feel I need to clarify that our first team meeting will be focused on UX discussions, engineering breakdown, and planning. I'm happy to find time for over-the-shoulder patch review during the rest of the week, separately with the interested engineers. Keep in mind that we have already quite a few platform patches in the works that may benefit from over-the-shoulder review more than the front-end patches.
Flags: needinfo?(paolo.mozmail)
Since this bug depends on bug 812894, I use github to manage the WIP patches:
https://github.com/weilonge/gecko/commits/seanlee/DownloadsPanel/Bug1282662

It includes the status for hover on MainArea [1] and Button [2][3].

[3] is just an experimental design which contains simpler CSS rules. Eventually, it will be merged into [2].

[1] https://github.com/weilonge/gecko/commit/0273e03993833ae5619b57ac0b18caa1e14ab06d.patch
[2] https://github.com/weilonge/gecko/commit/45b65dd9643004fbce4a44ffa68998b4a0245b68.patch
[3] https://github.com/weilonge/gecko/commit/fd4b7285e410e13c69971d2f2e600e0aba7a30be.patch
As a reference, this bug will change following UX items:
- “Cancelled”/”Failed”/”Paused” string only on “not hover”
- “Cancelled”/”Failed” show time and domain on “hover”
- Malware/uncommon/unwanted: “Show more information” on hover
- Show different strings when hovering on buttons
Hi Paolo,

You mentioned that "Starting..." state needs to be checked further in All Hands discussion. The only clue that I know is relative to 
[state="-1"] /* Starting (initial) */
[state="5"]  /* Starting (queued)  */

May I know how to test or verify the state? [state="-1"] and [state="5"] are not for the "Starting..." state in spec?

Thank you.

[1] http://searchfox.org/mozilla-central/rev/f680e72cc6579f90b992b63ca14d923d2afea612/browser/components/downloads/content/downloads.css#79-80
Flags: needinfo?(paolo.mozmail)
What I suggested is that maybe we don't ever show downloads in this state, so any change we make might not have any visible effect. For sure, two states exist instead of one because of how the downloads code evolved over time, and maybe even the remaining one is not needed anymore.

This is what I would do to verify if this is the case:
1) Look at the code to see if there is any path that can result in one or both of these states to be set,
2) If so, add some temporary console logging and run some downloads with various methods, to see if an item with that state is ever displayed in practice, and for how long.
Flags: needinfo?(paolo.mozmail)
Given that you're aggressively phasing out creation of new XUL extensions, will this change to the amount of information shown in the "Downloading" state be reversible using a WebExtensions API?

If not, it's a massive regression in efficiency for my primary use case (Glancing over the "time remaining" values and/or estimating the total bandwidth being consumed, given that I have to share 5Mbit DSL with three other family members) and another data point in favour of riding down from Aurora to ESR, then switching to something like Pale Moon.

That said, I do prefer the proposed new versions for the other 8 states and I believe I was the one to file the "Click to schedule opening when complete" chrome-parity bug.
I just realized I made an idiot mistake when reading the visual spec and mistook "Normal -> Hover" to mean "Normal/Current Version -> Version After Hover Rework".

Judging by the full thing on invision, my only actual regression issues with the new design are:

- The apparent removal of the time-remaining counter on the toolbar button means I'll need to acquire the button with my mouse, click, look at it, and then click again to close, rather than just glancing at it.

- I'll have to set my default downloads location to /tmp to ensure that auto-open doesn't start leaving trash lying around my system. (But, then, I had to do that anyway to get desired behaviour with WebExtensions-based things that save to disk, due to the limitations those APIs seem to impose.)
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review100622

::: browser/locales/en-US/chrome/browser/downloads/downloads.properties:21
(Diff revision 2)
>  statePaused=Paused
>  # LOCALIZATION NOTE (stateCanceled):
>  # Indicates that the download was canceled by the user.
>  stateCanceled=Canceled
> +# LOCALIZATION NOTE (stateOpenFile):
> +# Indicates that the download is able to open in system file browser.

Can you clarify this comment? 

Does the string describe a status (the file is open), like the string ID suggests, or an action (open this file)?
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review100622

> Can you clarify this comment? 
> 
> Does the string describe a status (the file is open), like the string ID suggests, or an action (open this file)?

This string is to describe an action to open this file. Should it be given another string ID rather with "state" prefix?
(In reply to Sean Lee [:seanlee][:weilonge] from comment #25)
> This string is to describe an action to open this file. Should it be given
> another string ID rather with "state" prefix?

Not necessarily, but it's important to explain as much as possible in the comment if that's the case.

For example, if we're actually opening the file

# Displayed when hovering a complete download, indicates that it's possible to
# open the file using an app available in the system.
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review100622

> This string is to describe an action to open this file. Should it be given another string ID rather with "state" prefix?

The comment is updated to a new one. I removed `state` prefix because "Open file" is an action rather than a state.
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

Looks good, one thing we should make sure to do is to disable the tooltips on the buttons, that are now redundant, but keeping the buttons labelled with the "aria-label" attribute for screen readers.
Attachment #8820731 - Flags: review?(paolo.mozmail)
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

In the Library window we should still show the full information including domain name and time, for example for cancelled downloads, with no change on hover.

In the Panel, there should be no details label for a completed download when the mouse is not hovering. The tooltip on the "Open File" or "File moved or missing" label should include the additional information like the domain name and time.

After trying out the patch, I've suggested the design change of not showing more details on hover in the Downloads Panel for cancelled, failed, and paused downloads, but keep them in the tooltip only. Let's wait for feedback on this.
Attachment #8817269 - Flags: review?(paolo.mozmail)
Hey Paolo,

The patch is updated based the latest UX design and add aria-label support instead of using tooltips.
Here is the try link for browser/components/downloads/test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ee5c22113f781887aa4f35e60280889e20f831a

There is no failed case in it. I will push another full try when this round is finished. Thank you.
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review106310

::: browser/components/downloads/DownloadsViewUI.jsm:256
(Diff revision 7)
>        } else if (this.download.error.becauseBlockedByParentalControls) {
> -        stateLabel = s.stateBlockedParentalControls;
> +        text = composeFullStatus(s.stateBlockedParentalControls,
> +                                 displayHost,
> +                                 displayDate);

The fact that this state wasn't included in any specification doesn't mean that we don't need to update it to make it consistent with the others!

It's likely you won't need the composeFullStatus helper, because you can join the strings after this code block like we did before.

::: browser/components/downloads/content/download.xml:51
(Diff revision 7)
>                               min="0"
>                               max="100"
>                               xbl:inherits="mode=progressmode,value=progress,paused=progresspaused"/>
> -          <xul:description class="downloadDetails"
> +          <xul:description class="downloadDetails downloadDetailsFull"
>                             crop="end"
> -                           xbl:inherits="value=status,tooltiptext=statusTip"/>
> +                           xbl:inherits="value=fullStatus,aria-label=fullStatus"/>

You don't need to repeat aria-label if the text is the same. However, for the Downloads View we still need to set the tooltip to show the full information on hover if the window is small and the text is truncated.

::: browser/components/downloads/content/download.xml:57
(Diff revision 7)
> +          <xul:description class="downloadDetails downloadDetailsNormal"
> +                           crop="end"
> +                           xbl:inherits="value=status"/>
> +          <xul:description class="downloadDetails downloadDetailsHover"
> +                           crop="end"
> +                           xbl:inherits="value=hoverStatus,aria-label=statusTip"/>

Since the aria-label is used mainly by screen readers, you don't need to set it in the mouse hover status.

::: browser/components/downloads/content/downloads.css:26
(Diff revision 7)
> +#downloadsListBox richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsFull,
> +#downloadsListBox richlistitem.download-state:hover > .downloadMainArea > .downloadContainer > description.downloadDetailsNormal,
> +#downloadsListBox richlistitem.download-state:not(:hover) > .downloadMainArea > .downloadContainer > description.downloadDetailsHover {
> +  display: none;
> +}
> +
> +#downloadsRichListBox richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsNormal,
> +#downloadsRichListBox richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsHover {
> +  display: none;
> +}
> +

Even if this is functional CSS, it's specific to whether this is the Downloads Panel or the Downloads View, so it may be simpler to move it to their specific theme CSS files instead of using the IDs of the richlistbox, just like we do for the action buttons.
Attachment #8817269 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review106316

This will need rebasing after the changes to part 1.

::: browser/components/downloads/content/download.xml:62
(Diff revision 4)
> +                             value="&cmd.showMac.label;"
> +                             aria-label="&cmd.showMac.label;"

As with part 1, no need to repeat the aria-label attribute. Moreover, this is only shown on mouse hover, so it isn't relevant to screen readers.

::: browser/locales/en-US/chrome/browser/downloads/downloads.dtd:92
(Diff revision 4)
> +<!ENTITY showMoreInformation.label        "Show more information">
> +

This is a repetition of the string from part 1, which would cause issues if localized differently. We should just modify the code so we continue to show the normal string in this case.

Also, the localization notes you added in part 1 were quite useful, can you do the same here?
Attachment #8820731 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review106316

> This is a repetition of the string from part 1, which would cause issues if localized differently. We should just modify the code so we continue to show the normal string in this case.
> 
> Also, the localization notes you added in part 1 were quite useful, can you do the same here?

Where should "Show more information" string be placed in? downloads.dtd or downloads.properties?

There are two possible solutions in my mind:
A. Keep it in downloads.dtd then modify DownloadsViewUI.jsm
B. Keep it in downloads.properties then modify downloads.xml

Actually I don't have a preference for this because I am not sure that how to implement both solutions.
May I have your suggestions?
Hey Paolo,

Thanks for reviewing this patch. The latest patch is with all fixes of the issue you raised except "Show more information" issue (comment 39).
Please review it again. Thank you.
(In reply to Sean Lee [:seanlee][:weilonge] from comment #39)
> Where should "Show more information" string be placed in? downloads.dtd or
> downloads.properties?
> 
> There are two possible solutions in my mind:
> A. Keep it in downloads.dtd then modify DownloadsViewUI.jsm
> B. Keep it in downloads.properties then modify downloads.xml
> 
> Actually I don't have a preference for this because I am not sure that how
> to implement both solutions.
> May I have your suggestions?

Looking at the two patches again, it seems to me that both "openFile" and "showMoreInformation" could be implemented in the DTD and controlled by CSS, since we already have the CSS for controlling the action buttons. This way, we don't need to place some hover state labels in the properties file and others in the DTD.

In the future we might want to go in a totally different direction, with a single element for the button that uses JavaScript instead of CSS to select which action to show. When that happens we'll likely move all these strings again from the DTD to the properties file, or even better to a l20n file if it's available by then.
Hey Paolo, "openFile" and "showMoreInformation" are moved into the DTD file. Please see the latest update again. Thanks.
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review107846

::: browser/components/downloads/DownloadsViewUI.jsm:180
(Diff revision 9)
>    /**
>     * Returns the text for the status line and the associated tooltip. These are
>     * returned by a single property because they are computed together. The
>     * result may be overridden by derived objects.
>     */
>    get statusTextAndTip() {
>      return this.rawStatusTextAndTip;
>    },
>  
>    /**
>     * Derived objects may call this to get the status text.
>     */
>    get rawStatusTextAndTip() {

Because we're now controlling which labels we show in the Library and the Panel using CSS, this indirection is not needed anymore.

In fact, the current patch leaves some unused code in the Library that doesn't make sense anymore and should be removed. There should be just one getter, and should be named differently, for example "statusLabels".

::: browser/components/downloads/DownloadsViewUI.jsm:195
(Diff revision 9)
>     * Derived objects may call this to get the status text.
>     */
>    get rawStatusTextAndTip() {
>      let s = DownloadsCommon.strings;
>  
>      let text = "";

nit: rename "text" to "status" for consistency, also in the returned object.

::: browser/locales/en-US/chrome/browser/downloads/downloads.properties:56
(Diff revision 9)
> +# Displayed when hovering a complete download which is not at the original
> +# folder.

The "when hovering" part is not correct.

::: browser/themes/shared/downloads/allDownloadsViewOverlay.inc.css:24
(Diff revision 9)
> +#downloadsRichListBox > richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsNormal,
> +#downloadsRichListBox > richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsHover {
> +  display: none;
> +}

This complexity is totally unneeded.

.downloadDetailsNormal,
.downloadDetailsHover {
  display: none;
}

Also, move this after the ".downloadDetails" rule.

::: browser/themes/shared/downloads/downloads.inc.css:179
(Diff revision 9)
> +richlistitem.download-state > .downloadMainArea > .downloadContainer > description.downloadDetailsFull,
> +richlistitem.download-state:hover > .downloadMainArea > .downloadContainer > description.downloadDetailsNormal,
> +richlistitem.download-state:not(:hover) > .downloadMainArea > .downloadContainer > description.downloadDetailsHover {
> +  display: none;
> +}

Same as above, and in the last two rules use @item@ and remove the "description" tag.
Attachment #8817269 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review107870

Also here, the CSS can be simplified, so I haven't reviewed it yet.

When testing, I found an issue for which the wrong label is shown in the Panel when hovering over the toolbarseparator.

For action buttons in the Library, we should either use the same mouse hover behavior as the Downloads Panel, or continue to display the tooltip. To implement the latter, we would need to use JavaScript to copy the "aria-label" attribute to the "tooltip" attribute.

::: browser/components/downloads/content/downloads.js:1002
(Diff revision 6)
> +  onDownloadButtonMouseOver(aEvent) {
> +    aEvent.target.parentNode.parentNode.classList.add("downloadShowIconDesc");
> +  },
> +
> +  onDownloadButtonMouseOut(aEvent) {
> +    aEvent.target.parentNode.parentNode.classList.remove("downloadShowIconDesc");
> +  },
> +

Using "parentNode" in this way is not robust in case of future changes to the item structure.

We could iterate like we do in other parts of the code, but a slightly faster approach, after rebasing on top of bug 991965, could be to use the current onDownloadMouseOver event.

The "event.target" there is already checked and it should point to the <richlistitem>. You can check whether the "event.originalTarget" is one of the buttons. I believe it's fine to check the class of its ".parentNode" in this case, moving up just one level.

::: browser/locales/en-US/chrome/browser/downloads/downloads.dtd:98
(Diff revision 6)
> +<!-- LOCALIZATION NOTE (openFile.label):
> +     Displayed when hovering a complete download, indicates that it's possible to
> +     open the file using an app available in the system.
> +     -->
> +<!ENTITY openFile.label        "Open file">
> +
> +<!-- LOCALIZATION NOTE (retryDownload.label):
> +     Displayed when hovering a download which is able to be retried by users,
> +     indicates that it's possible to download this file again.
> +     -->
> +<!ENTITY retryDownload.label              "Retry download">
> +
> +<!-- LOCALIZATION NOTE (cancelDownload.label):
> +     Displayed when hovering a download which is able to be cancelled by users,
> +     indicates that it's possible to cancel and stop the download.
> +     -->
> +<!ENTITY cancelDownload.label             "Cancel download">
> +

Capitalization is inconsistent. This is especially visible when moving the mouse between the "Open file" and the "Show In Finder" commands.

We may need feedback from the user experience team on which approach to choose.

Option 1 is renaming "Show In Finder" to "Show in Finder", which means adding a separate string with a different capitalization than the one used in the menu.

Option 2 is renaming "Retry download" to "Retry Download", which means using title capitalization for all commands, even when they're displayed as labels.

The "Show more information" and "File moved or missing" labels don't look like commands to me, and are displayed regardless of mouse hover, so we may use different capitalization rules for those.
Attachment #8820731 - Flags: review?(paolo.mozmail)
(In reply to :Paolo Amadini from comment #48)
> The "Show more information" and "File moved or missing" labels don't look
> like commands to me, and are displayed regardless of mouse hover, so we may
> use different capitalization rules for those.

Correction, for the record, "Show more information" is displayed on mouse hover so I don't see a reason for which we couldn't use title capitalization for this string as well. To clarify, I'm not saying we should change it either, the current capitalization feels better to me. I'm fine with both options.
Hey Paolo,

The latest patch is with the fixes based on your comments except the title capitalization part. I will discuss this with UX on Thursday. Could you review the patch first for the CSS refine and "statusLabels" function parts? Thank you.
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review109878

A few things still unaddressed, also please determine what to do with capitalization before asking for review again.

::: browser/components/downloads/DownloadsViewUI.jsm:214
(Diff revision 10)
>        // transferred, for example "Paused -  1.1 MB".
> -      text = s.statusSeparatorBeforeNumber(s.statePaused, transfer);
> +      status = s.statusSeparatorBeforeNumber(s.statePaused, transfer);
> +      hoverStatus = status;
>      } else if (!this.download.succeeded && !this.download.canceled &&
>                 !this.download.error) {
> -      text = s.stateStarting;
> +      status = s.stateStarting;

Add "hoverStatus = status;" for code correctness, even though we determined this state is probably not used normally.

::: browser/components/downloads/DownloadsViewUI.jsm:231
(Diff revision 10)
>            stateLabel = s.sizeWithUnits(size, unit);
>          } else {
>            // History downloads may not have a size defined.
>            stateLabel = s.sizeUnknown;
>          }
> +        status = s.stateCompleted;

You didn't fix the issue for which the wrong label is shown in the Panel when hovering over the toolbarseparator. I believe that adding "hoverStatus = status;" here should at least make it consistent.
Bug 1335403 is on file to determine what to do with the toolbarseparator anyways.

::: browser/components/downloads/content/download.xml:49
(Diff revision 10)
> -          <xul:description class="downloadDetails"
> +          <xul:description class="downloadDetails downloadDetailsFull"
>                             crop="end"
> -                           xbl:inherits="value=status,tooltiptext=statusTip"/>
> +                           xbl:inherits="value=fullStatus,tooltiptext=fullStatus"/>
> +          <xul:description class="downloadDetails downloadDetailsNormal"
> +                           crop="end"
> +                           xbl:inherits="value=status"/>
> +          <xul:description class="downloadDetails downloadDetailsHover"
> +                           crop="end"
> +                           xbl:inherits="value=hoverStatus"/>

nit: can you use the order normal, hover, full, which is the same one used by the rest of the code?

::: browser/themes/shared/downloads/downloads.inc.css:288
(Diff revision 10)
> +@item@ > .downloadMainArea > .downloadContainer > .downloadDetailsFull,
> +@item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
> +@item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadDetailsHover {
> +  display: none;
> +}
> +

Obviously, the first selector can just be ".downloadDetailsFull". Also, this goes after the ".downloadDetails" rule, line 235 in this version.
Attachment #8817269 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review109886

(In reply to :Paolo Amadini from comment #48)
> For action buttons in the Library, we should either use the same mouse hover
> behavior as the Downloads Panel, or continue to display the tooltip. To
> implement the latter, we would need to use JavaScript to copy the
> "aria-label" attribute to the "tooltip" attribute.

You didn't address this review comment either.

::: browser/themes/shared/downloads/downloads.inc.css:290
(Diff revision 7)
>    color: white;
>  }
>  
> +@itemFinished@ > .downloadMainArea:not(hover) > .downloadContainer > .downloadButtonDesc > .downloadOpenFile,
> +@itemNotFinished@ > .downloadMainArea > .downloadContainer > .downloadButtonDesc > .downloadOpenFile,
> +@itemFinished@ > .downloadMainArea:hover > .downloadContainer > .downloadButtonDesc > .downloadShow,

I'm not quite sure what you're trying to accomplish here. It looks like you're using the existing generic visibility class for the labels, for example ".downloadShow", in which case you definitely don't need specific rules to define the visibility of each label again.

Also, some rules are negative and set "display: none;", others are positive and reset this to "display: inherit;", but there doesn't seem to be a consistent logic about when you use one and when you use the other.

Can you clean up the situation here, and ask for review again after you discuss the capitalization issue?
Attachment #8820731 - Flags: review?(paolo.mozmail)
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review109878

> You didn't fix the issue for which the wrong label is shown in the Panel when hovering over the toolbarseparator. I believe that adding "hoverStatus = status;" here should at least make it consistent.
> Bug 1335403 is on file to determine what to do with the toolbarseparator anyways.

Thanks for the suggestion. I've included it in the latest patch and verified it in the cases of Downloading, Canceled, Completed, etc.
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review109886

After considering the performance, I add the tooltiptext of action buttons into download.xml but exclude them in Downloads Panel (downloads.js). There are two reasons to remove the tooltiptext in Downloads Panel rather than add these in Downloads Library:
1. IMHO, the JS code execution is expensive, so I would like to treat any JS code exection as the special cases.
2. The items in Library can be many more than Downloads Panel, so the performance cost of removing the tooltiptext in Downloads Panel is lesser than the adding one.

> I'm not quite sure what you're trying to accomplish here. It looks like you're using the existing generic visibility class for the labels, for example ".downloadShow", in which case you definitely don't need specific rules to define the visibility of each label again.
> 
> Also, some rules are negative and set "display: none;", others are positive and reset this to "display: inherit;", but there doesn't seem to be a consistent logic about when you use one and when you use the other.
> 
> Can you clean up the situation here, and ask for review again after you discuss the capitalization issue?

All CSS rules are refined to use negative rules (display:none) and add some comments to explain the logic.
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review107870

> Capitalization is inconsistent. This is especially visible when moving the mouse between the "Open file" and the "Show In Finder" commands.
> 
> We may need feedback from the user experience team on which approach to choose.
> 
> Option 1 is renaming "Show In Finder" to "Show in Finder", which means adding a separate string with a different capitalization than the one used in the menu.
> 
> Option 2 is renaming "Retry download" to "Retry Download", which means using title capitalization for all commands, even when they're displayed as labels.
> 
> The "Show more information" and "File moved or missing" labels don't look like commands to me, and are displayed regardless of mouse hover, so we may use different capitalization rules for those.

After discussing this with Bryant, he thinks it's fine to choose option 2 currently. Since UX team need some time to discuss the capitalization in detail, we can file a follow-up bug to refine this if needed.
Comment on attachment 8817269 [details]
Bug 1282662 - Part 1: Redesign the detail description of Download item in normal or hover cases.;

https://reviewboard.mozilla.org/r/97628/#review110324

Part 1 looks good!
Attachment #8817269 - Flags: review?(paolo.mozmail) → review+
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review110326

::: browser/components/downloads/content/download.xml:86
(Diff revision 8)
>        </xul:hbox>
>        <xul:toolbarseparator />
>        <xul:stack class="downloadButtonArea">
>          <xul:button class="downloadButton downloadCancel downloadIconCancel"
>                      tooltiptext="&cmd.cancel.label;"
> +                    aria-label="&cmd.cancel.label;"

This approach works for me, but you don't need to repeat the "aria-label" attribute in the XUL file. Since you're manipulating the item from JavaScript anyways, you can read the value of the "tooltiptext" attribute and move it to "aria-label".

::: browser/components/downloads/content/downloads.js:935
(Diff revision 8)
> +    setTimeout(() => {
> +      let buttonArea = element.ownerDocument
> +                       .getAnonymousElementByAttribute(element,
> +                                                       "class",
> +                                                       "downloadButtonArea");
> +      if (buttonArea) {
> +        for (let i = 0; i < buttonArea.children.length; i++) {
> +          let button = buttonArea.children[i];
> +          button.attributes.removeNamedItem("tooltiptext");
> +        }
> +      }
> +    }, 0);

I'd have expected a code comment explaining why the setTimeout is necessary. I guess you're using it to work around issues with late XBL binding, but this isn't robust anyways.

A better approach that doesn't require setTimeout is to swap the attributes in the onDownloadMouseOver handler, if the "tooltip" attribute is still present.

Please use the getAttribute, setAttribute, and removeAttribute methods, and not the "attributes" property.

::: browser/components/downloads/content/downloads.js:1053
(Diff revision 8)
>    /**
>     * Mouse listeners to handle selection on hover.
>     */
>    onDownloadMouseOver(aEvent) {
> +    if (aEvent.originalTarget.classList.contains("downloadButton")) {
> +      aEvent.target.classList.add("downloadShowIconDesc");

downloadShowIconDesc -> downloadHoveringButton

::: browser/themes/shared/downloads/downloads.inc.css:235
(Diff revision 8)
> +/* The class usage in the general cases is like this:
> +   .downloadDetailsNormal: non-hovering case
> +   .downloadDetailsHover: hovering on downloadMainArea
> +   .downloadButtonDesc: hovering on action buttons. The item comes along with
> +                        .downloadShowIconDesc as well.
> +*/
>  @item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
>  @item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
> +@item@:not(.downloadShowIconDesc) > .downloadMainArea > .downloadContainer > .downloadButtonDesc,
> +@item@.downloadShowIconDesc > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
> +@item@.downloadShowIconDesc > .downloadMainArea > .downloadContainer > .downloadDetailsHover,

I tested this, and the "File moved or missing" case is handled incorrectly.

Thanks for creating the groups. For clarity, I'd place the "display: none;" block at the end of each group.

Also, place .downloadDetailsFull in its own separate group, since it's never shown and so it's logically separate from the rules.

The comments can definitely be made clearer.

In the future, I look forward to having the code refactored with just one element for all the buttons and labels, which is a different model than the one we're using now.
Attachment #8820731 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review110326

> I'd have expected a code comment explaining why the setTimeout is necessary. I guess you're using it to work around issues with late XBL binding, but this isn't robust anyways.
> 
> A better approach that doesn't require setTimeout is to swap the attributes in the onDownloadMouseOver handler, if the "tooltip" attribute is still present.
> 
> Please use the getAttribute, setAttribute, and removeAttribute methods, and not the "attributes" property.

These codes moving tooltiptext to aria-label have been moved into onDownloadMouseOver handler, and it works with much simpler codes! Thanks for the suggestion.

> downloadShowIconDesc -> downloadHoveringButton

I also rename downloadButtonDesc to downloadButtonLabels.

> I tested this, and the "File moved or missing" case is handled incorrectly.
> 
> Thanks for creating the groups. For clarity, I'd place the "display: none;" block at the end of each group.
> 
> Also, place .downloadDetailsFull in its own separate group, since it's never shown and so it's logically separate from the rules.
> 
> The comments can definitely be made clearer.
> 
> In the future, I look forward to having the code refactored with just one element for all the buttons and labels, which is a different model than the one we're using now.

"File moved or missing" is been fixed in the latest patch. "display: none;" is also applied to each group. Could you give a suggestion that how to make the comment clearer?
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review111054

(In reply to Sean Lee [:seanlee][:weilonge] from comment #64)
> I also rename downloadButtonDesc to downloadButtonLabels.

Good idea, the plural clarifies the meaning.

::: browser/themes/shared/downloads/downloads.inc.css:235
(Diff revision 9)
> +/* The class usage in the general cases is like this:
> +   .downloadDetailsNormal: non-hovering case
> +   .downloadDetailsHover: hovering on downloadMainArea
> +   .downloadButtonLabels: hovering on action buttons. The item comes along with
> +                          .downloadHoveringButton as well.
> +*/
>  @item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
>  @item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
> +@item@:not(.downloadHoveringButton) > .downloadMainArea > .downloadContainer > .downloadButtonLabels,
> +@item@.downloadHoveringButton > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
> +@item@.downloadHoveringButton > .downloadMainArea > .downloadContainer > .downloadDetailsHover {
> +  display: none;
> +}
> +
> +/* state="1" is a special case due to "Open File" label for hovering case, so
> +   downloadOpenFile is treated as downloadDetailsHover with some tweaks.
> +   When the download does not exist, it never shows "Open File" label but
> +  "File moved or missing" in both normal and hovering cases.
> +*/
> +@itemNotFinished@ > .downloadMainArea > .downloadContainer > .downloadOpenFile,
> +@itemFinished@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadOpenFile,
> +@itemFinished@:hover > .downloadMainArea:not(:hover) > .downloadContainer > .downloadOpenFile,
> +@itemFinished@:not([exists]) > .downloadMainArea > .downloadContainer > .downloadOpenFile,
> +@itemFinished@[exists]:hover > .downloadMainArea:hover > .downloadContainer > .downloadDetailsHover {
> +  display: none;
> +}
> +
> +/* state="8" is another special case due to "Show more information" label for
> +   hovering case, and downloadShowMoreInfo is treated as downloadDetailsHover
> +   and downloadButtonLabels with some tweaks.
> +*/
> +richlistitem:not([state="8"]) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo,
> +richlistitem[state="8"]:not(:hover) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo,
> +richlistitem[state="8"]:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
> +richlistitem[state="8"] > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
> +richlistitem[state="8"] > .downloadMainArea > .downloadContainer > .downloadButtonLabels {
> +  display: none;
> +}
> +
>  .downloadDetailsFull {
>    display: none;
>  }

(In reply to Sean Lee [:seanlee][:weilonge] from comment #64)
> Could you give a suggestion that how to make the comment clearer?

When the code has to implement a logic that is not trivial, and this case is a good example, writing clear code comments to explain what you're doing often means that the code itself becomes clearer.

Following the logic step by step to write the comments, from the simplest case to the most complex, I found out that various rules could be simplified.

I've pasted the result here. Note how each block has the same structure. The comment says "When X, instead of A we display B", and the following rules are always in order, first for hiding A, then for hiding B in the opposite case.

When integrating this portion with your current patch, please read it carefully and check that I didn't forget anything, then test each case again before requesting the last review. Thanks!

/* The following rules control which message is shown under the name of the
   download, using a set of elements that share the class ".downloadDetails".
   At any given time, only one of these elements is displayed. We use a set of
   rules to hide the elements that shouldn't be displayed in each case. */

/* The full status message is only displayed in the Downloads View. */
.downloadDetailsFull {
  display: none;
}

/* When hovering the mouse pointer over the item, instead of the normal message
   we display a more detailed one. */
@item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
@item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadDetailsHover {
  display: none;
}

/* When hovering the action button in particular, instead of the usual hover
   message we display the command associated with the button. */
@item@.downloadHoveringButton > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
@item@:not(.downloadHoveringButton) > .downloadMainArea > .downloadContainer > .downloadButtonLabels {
  display: none;
}

/* When hovering the main area of a finished download whose target exists,
   instead of the usual hover message we display the "Open File" command. */
@itemFinished@[exists] > .downloadMainArea:hover > .downloadContainer > .downloadDetailsHover,
@itemNotFinished@ > .downloadMainArea > .downloadContainer > .downloadOpenFile,
@item@:not([exists]) > .downloadMainArea > .downloadContainer > .downloadOpenFile,
.downloadMainArea:not(:hover) > .downloadContainer > .downloadOpenFile {
  display: none;
}

/* When hovering items blocked by Application Reputation, instead of the other
   hover messages we display the "Show more information" label. */
@item@[verdict] > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
@item@[verdict] > .downloadMainArea > .downloadContainer > .downloadButtonLabels,
@item@:not([verdict]) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo,
@item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo {
  display: none;
}
Attachment #8820731 - Flags: review?(paolo.mozmail)
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review111054

> (In reply to Sean Lee [:seanlee][:weilonge] from comment #64)
> > Could you give a suggestion that how to make the comment clearer?
> 
> When the code has to implement a logic that is not trivial, and this case is a good example, writing clear code comments to explain what you're doing often means that the code itself becomes clearer.
> 
> Following the logic step by step to write the comments, from the simplest case to the most complex, I found out that various rules could be simplified.
> 
> I've pasted the result here. Note how each block has the same structure. The comment says "When X, instead of A we display B", and the following rules are always in order, first for hiding A, then for hiding B in the opposite case.
> 
> When integrating this portion with your current patch, please read it carefully and check that I didn't forget anything, then test each case again before requesting the last review. Thanks!
> 
> /* The following rules control which message is shown under the name of the
>    download, using a set of elements that share the class ".downloadDetails".
>    At any given time, only one of these elements is displayed. We use a set of
>    rules to hide the elements that shouldn't be displayed in each case. */
> 
> /* The full status message is only displayed in the Downloads View. */
> .downloadDetailsFull {
>   display: none;
> }
> 
> /* When hovering the mouse pointer over the item, instead of the normal message
>    we display a more detailed one. */
> @item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
> @item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadDetailsHover {
>   display: none;
> }
> 
> /* When hovering the action button in particular, instead of the usual hover
>    message we display the command associated with the button. */
> @item@.downloadHoveringButton > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
> @item@:not(.downloadHoveringButton) > .downloadMainArea > .downloadContainer > .downloadButtonLabels {
>   display: none;
> }
> 
> /* When hovering the main area of a finished download whose target exists,
>    instead of the usual hover message we display the "Open File" command. */
> @itemFinished@[exists] > .downloadMainArea:hover > .downloadContainer > .downloadDetailsHover,
> @itemNotFinished@ > .downloadMainArea > .downloadContainer > .downloadOpenFile,
> @item@:not([exists]) > .downloadMainArea > .downloadContainer > .downloadOpenFile,
> .downloadMainArea:not(:hover) > .downloadContainer > .downloadOpenFile {
>   display: none;
> }
> 
> /* When hovering items blocked by Application Reputation, instead of the other
>    hover messages we display the "Show more information" label. */
> @item@[verdict] > .downloadMainArea > .downloadContainer > .downloadDetailsHover,
> @item@[verdict] > .downloadMainArea > .downloadContainer > .downloadButtonLabels,
> @item@:not([verdict]) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo,
> @item@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadShowMoreInfo {
>   display: none;
> }

Thank you very much to simplify these complex rules into much simpler ones!!

Since these rules are not trivial, what I understand the changes you made are listed here to make sure your thoughts are understood by me correctly.
A. The following rules are equivalent to “@item@:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal”:
@item@.downloadHoveringButton > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
richlistitem[state="8"]:hover > .downloadMainArea > .downloadContainer > .downloadDetailsNormal,
B. Split the action button rules into an isolated group.
C. richlistitem[state="8”] are replaced with @item@[verdict].
D. “@itemFinished@:not(:hover) > .downloadMainArea > .downloadContainer > .downloadOpenFile” is useless if “.downloadMainArea:not(:hover) > .downloadContainer > .downloadOpenFile “ is applied.

The hovering labels of Action Button, Main Area, and separator are verified in the following cases:
1. Completed
2. Malware/Uncommon/Unwanted - the downloaded case.
3. Malware/Uncommon/Unwanted - file-opened case.
4. Deleted or moved
5. Downloading
6. Canceled
7. Paused

Please help to review it again. Thanks.
Comment on attachment 8820731 [details]
Bug 1282662 - Part 2: Implement the detail description in hover button case.;

https://reviewboard.mozilla.org/r/100184/#review111546

(In reply to Sean Lee [:seanlee][:weilonge] from comment #68)
> Since these rules are not trivial, what I understand the changes you made
> are listed here to make sure your thoughts are understood by me correctly.

Mostly what you said, in general if you read rules from right to left you can remove selectors as soon as you find cases that don't need to be checked.

Thanks for the testing!

::: browser/themes/shared/downloads/downloads.inc.css:277
(Diff revision 10)
>  .downloadDetailsFull {
>    display: none;
>  }
>  

This is a leftover, can be removed since it's now repeated above.
Attachment #8820731 - Flags: review?(paolo.mozmail) → review+
Thanks a lot for reviewing this patch!

(In reply to :Paolo Amadini from comment #69)
> Comment on attachment 8820731 [details]
> Bug 1282662 - Part 2: Implement the detail description in hover button case.;
> 
> https://reviewboard.mozilla.org/r/100184/#review111546
> 
> (In reply to Sean Lee [:seanlee][:weilonge] from comment #68)
> > Since these rules are not trivial, what I understand the changes you made
> > are listed here to make sure your thoughts are understood by me correctly.
> 
> Mostly what you said, in general if you read rules from right to left you
> can remove selectors as soon as you find cases that don't need to be checked.
> 
> Thanks for the testing!
> 
> ::: browser/themes/shared/downloads/downloads.inc.css:277
> (Diff revision 10)
> >  .downloadDetailsFull {
> >    display: none;
> >  }
> >  
> 
> This is a leftover, can be removed since it's now repeated above.
It's been removed in the final patch.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef2825d0cbfa
Part 1: Redesign the detail description of Download item in normal or hover cases.; r=Paolo
https://hg.mozilla.org/integration/autoland/rev/e80e0e5b07db
Part 2: Implement the detail description in hover button case.; r=Paolo
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ef2825d0cbfa
https://hg.mozilla.org/mozilla-central/rev/e80e0e5b07db
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
https://hg.mozilla.org/mozilla-central/diff/ef2825d0cbfa/browser/locales/en-US/chrome/browser/downloads/downloads.properties
> +# LOCALIZATION NOTE (fileMovedOrMissing):
> +# Displayed when a complete download which is not at the original folder.
> +fileMovedOrMissing=File moved or missing

What exactly "Displayed when a complete download which is not at the original folder" and "File moved or missing" means? Is there some other state for moved file than missing (like moved to known location)?
I doubt that Firefox can keep track of a moved file. Having said that, wording on that comment should definitely be improved.

I assume the meaning is "Displayed on a complete download when the file is not available anymore in the original position"?
I noticed some things are not as in the attachment.
- PAUSED - while hovering on thread status/action time/source is not displayed
- DOWNLOADING - while hovering "Open when done" is not displayed

Cheers!
Hi Cristian,

This behavior is by UX design. Bryant will help to clarify your concern. Thanks.

Hi Bryant,

Could you help to clarify Cristian's concern? Thanks!
(In reply to Cristian Comorasu [:CristiComo] from comment #77)
> I noticed some things are not as in the attachment.
> - PAUSED - while hovering on thread status/action time/source is not
> displayed
> - DOWNLOADING - while hovering "Open when done" is not displayed
Flags: needinfo?(bmao)
hi Cristian,

We've made some adjustment to the design after we uploaded the attachment, and I should be able to update the attachments by the end of next week. Bur let me address the issues first: 

(In reply to Cristian Comorasu [:CristiComo] from comment #77)
> I noticed some things are not as in the attachment.
> - PAUSED - while hovering on thread status/action time/source is not
> displayed
Yes, we will keep the information the same in both normal and hover state. So there won't be status/action time/source info on the thread while hovering.

> - DOWNLOADING - while hovering "Open when done" is not displayed
Yes, we removed this feature from the MVP design spec. So when hovering, the information will be the same as the normal state.
> 
> Cheers!

thanks.
Flags: needinfo?(bmao)
Hi Cristian,

Just saw the "Pre-Beta Sign-Off report" and thanks for the support!
This one is mentioned in it and as the above comment (by UX Bryant) clarified, the behavior is correct.
Would you kindly check and see if we are good for this one, or prefer to verify again after the update UX spec is attached?

Thanks!
Flags: needinfo?(cristian.comorasu)
Hello Wesly!
I would like to re-verify this issue once the update UX spec is attached. 
Thank you!
Flags: needinfo?(cristian.comorasu)
(In reply to Cristian Comorasu [:CristiComo] from comment #81)
> Hello Wesly!
> I would like to re-verify this issue once the update UX spec is attached. 
> Thank you!

Thanks and no problem, Cristian!

Hi Bryant, would you help let us know once the updated UX spec is ready? Thanks. (thought you are working on one these days?)
Flags: needinfo?(bmao)
(In reply to Wesly Huang (Firefox EPM) from comment #82)
> Hi Bryant, would you help let us know once the updated UX spec is ready?
> Thanks. (thought you are working on one these days?)

Hi Cristian, 

Just realized the latest UX spec (including needed information here) has been updated in https://bugzilla.mozilla.org/show_bug.cgi?id=1269956#c28.

Would you help check when you have time? Thanks!
Flags: needinfo?(bmao) → needinfo?(cristian.comorasu)
Hi Wesly!

I just verified this issue using Fx 54.0a2 (build ID: 20170410004019) on Windows 10 x64, Mac OS X 10.11.6 and Ubuntu 14.04 LTS. I can confirm the behavior meets the documentation.

Cheers!
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
Flags: needinfo?(cristian.comorasu)
Thanks Cristian!
Release Note Request (optional, but appreciated)
[Why is this notable]: There are UI changes as mentioned in the suggesting wording below. It's related to file download, which is a frequently used feature.
[Affects Firefox for Android]: No
[Suggested wording]: Redesign the detailed description of download items for both normal and hover cases, and add information in hover button cases, in the download panel.
[Links (documentation, blog post, etc)]: page 18~20 of this UX spec: https://bug1269956.bmoattachments.org/attachment.cgi?id=8855701
relnote-firefox: --- → ?
Added to Fx54 (Beta) release notes.
relnote-firefox: ? → 54+
why can't we just not have an option to revert this change? showing full details in download window or hover window how we use to have saves a lot of time and trouble. instead of having to go to library. will this get reverted back in the future or at least an option to revert back?
Depends on: 1354568
Bring back all download information like it was. Please. Now it's too confusing. To many clicks to get what is needed.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: