1452629 - Avoid redundant elements in the "download" widget

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, enhancement, P1)

Description

[:Paolo Amadini]

The "download" widget is used by the Downloads Panel and the Downloads View, and it currently uses different button elements for each possible action.

These elements are shown or hidden using CSS styles that use the legacy "state" attribute. This is more difficult to understand and maintain than a solution based on changing the associated action in JavaScript.

Converting this to a Custom Element or a plain class will be much easier if all the elements don't have to be created programmatically.

Comment 1

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 1 - Avoid redundant button elements in the "download" widget. r=mak

Comment 2

[:Paolo Amadini]

I'll land this after the merge. I'll use the same approach for the status labels, and maybe the context menu as well.

Comment 3

[:Paolo Amadini]

Comment on attachment 9004529 [details]
Bug 1452629 - Part 1 - Avoid redundant button elements in the "download" widget. r=mak

In "downloadsStrings.inc.xul", I've assigned the attribute names using identifiers that resemble Fluent conventions, so we can just keep the same l10nId in the code. Do the identifier names look fine to you?

Comment 4

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 2 - Avoid redundant button label elements in the "download" widget. r=mak

Depends on D4441

Comment 5

[:Paolo Amadini]

Comment on attachment 9004554 [details]
Bug 1452629 - Part 2 - Avoid redundant button label elements in the "download" widget. r=mak

Here I've added new identifiers with "-description" at the end, which are used on a different element. For one of them, I've added a new identifier even if the string is the same, since they're used in a different context.

Comment 6

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 3 - Remove the "downloadOpenFile" description element. r=mak

The "Open File" message is now correctly displayed when hovering the one-pixel-wide clickable area around the action button.

Depends on D4447

Comment 7

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 4 - Remove the "downloadShowMoreInfo" description element. r=mak

Depends on D4459

Comment 8

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 5 - Remove the "downloadDetailsFull" description element. r=mak

Depends on D4460

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9004529 [details]
Bug 1452629 - Part 1 - Avoid redundant button elements in the "download" widget. r=mak

Zibi Braniecki [:gandalf][:zibi] has approved the revision.

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9004554 [details]
Bug 1452629 - Part 2 - Avoid redundant button label elements in the "download" widget. r=mak

Zibi Braniecki [:gandalf][:zibi] has approved the revision.

Comment 11

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca3233ac9c64efe6c39eb9e2ff3a0090c1d27b31

Comment 12

[:Paolo Amadini]

Attachment: Bug 1452629 - Part 6 - Refactor the user interface update code of download elements. r=mak

Depends on D4461

Comment 13

[:Paolo Amadini]

I've added a refactoring commit to simplify the state handling after the changes made here, so we have just one instead of three different blocks that check the download state.

At the moment, I've only posted part of the refactoring, so the review can more easily follow each step, but I'll do more work and update Part 6 afterwards.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9004529 [details]
Bug 1452629 - Part 1 - Avoid redundant button elements in the "download" widget. r=mak

Zibi Braniecki [:gandalf][:zibi] has been removed from the revision.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9004554 [details]
Bug 1452629 - Part 2 - Avoid redundant button label elements in the "download" widget. r=mak

Zibi Braniecki [:gandalf][:zibi] has been removed from the revision.

Comment 16

[Marco Bonardo [:mak]]

Comment on attachment 9004529 [details]
Bug 1452629 - Part 1 - Avoid redundant button elements in the "download" widget. r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 17

[Marco Bonardo [:mak]]

Comment on attachment 9004554 [details]
Bug 1452629 - Part 2 - Avoid redundant button label elements in the "download" widget. r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 18

[Marco Bonardo [:mak]]

Comment on attachment 9004597 [details]
Bug 1452629 - Part 3 - Remove the "downloadOpenFile" description element. r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 19

[Marco Bonardo [:mak]]

Comment on attachment 9004598 [details]
Bug 1452629 - Part 4 - Remove the "downloadShowMoreInfo" description element. r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 20

[Marco Bonardo [:mak]]

(In reply to :Paolo Amadini from comment #13)
> At the moment, I've only posted part of the refactoring, so the review can
> more easily follow each step, but I'll do more work and update Part 6
> afterwards.

So, what's the current status of part 6?

Comment 21

[Marco Bonardo [:mak]]

Comment on attachment 9004599 [details]
Bug 1452629 - Part 5 - Remove the "downloadDetailsFull" description element. r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 22

[:Paolo Amadini]

(In reply to Marco Bonardo [::mak] from comment #20)
> So, what's the current status of part 6?

I've just completed the refactoring and I'm posting the code for review right now. It may use some more code comments, let me know.

Comment 23

[:Paolo Amadini]

I'll probably move the refactoring to a different bug, since it also does minor changes to New Tab Page code. Anyways, these will all land after the merge.

Comment 24

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ce4d95d7d746a7e6eeda0980b297dbeeef2d3b4

Comment 25

[:Paolo Amadini]

In the latest version I've removed the string cache, following Zibi's recommendation.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58fc7f243f76
Part 1 - Avoid redundant button elements in the "download" widget. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/de4eb68568a0
Part 2 - Avoid redundant button label elements in the "download" widget. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/4aca3cccf16f
Part 3 - Remove the "downloadOpenFile" description element. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/48e3334d170d
Part 4 - Remove the "downloadShowMoreInfo" description element. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/1de95b7783f0
Part 5 - Remove the "downloadDetailsFull" description element. r=mak

Comment 27

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/58fc7f243f76
https://hg.mozilla.org/mozilla-central/rev/de4eb68568a0
https://hg.mozilla.org/mozilla-central/rev/4aca3cccf16f
https://hg.mozilla.org/mozilla-central/rev/48e3334d170d
https://hg.mozilla.org/mozilla-central/rev/1de95b7783f0