Closed Bug 1443971 Opened 4 years ago Closed 4 years ago

Remove allDownloadsViewOverlay.xul

Categories

(Firefox :: Downloads Panel, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: bdahl, Assigned: bdahl)

References

Details

Attachments

(1 file)

After the work in bug 1443901, this overlay will be used in places.xul and contentAreaDownloadsView.xul. My plan is to move the commandset and menuppop into include files, inline downloadsRichListBox (it only defines attributes), and inline the scripts and DTDs.

It looks like this will depend on the removal of the macBrowserOverlay so browser.js can be loaded before allDownloadsViewOverlay.js to avoid the conflict with DownloadsView.
Blocks: 1445764
Comment on attachment 8960386 [details]
Bug 1443971 - Remove allDownloadsViewOverlay.xul.

https://reviewboard.mozilla.org/r/229170/#review235412

::: browser/components/downloads/content/contentAreaDownloadsView.xul:44
(Diff revision 1)
> -    <richlistbox id="downloadsRichListBox"/>
> +    <richlistbox flex="1"
> +             seltype="multiple"
> +             id="downloadsRichListBox" context="downloadsContextMenu"
> +             onscroll="return this._placesView.onScroll();"
> +             onkeypress="return this._placesView.onKeyPress(event);"
> +             ondblclick="return this._placesView.onDoubleClick(event);"
> +             oncontextmenu="return this._placesView.onContextMenu(event);"
> +             ondragstart="this._placesView.onDragStart(event);"
> +             ondragover="this._placesView.onDragOver(event);"
> +             ondrop="this._placesView.onDrop(event);"
> +             onfocus="goUpdateDownloadCommands();"
> +             onselect="this._placesView.onSelect();"
> +             onblur="goUpdateDownloadCommands();"/>

Given the high number of attributes, I'd move this to an include file as well.

::: browser/components/downloads/content/download.xml:19
(Diff revision 1)
>            xmlns:xbl="http://www.mozilla.org/xbl">
>  
>    <binding id="download"
>             extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
>      <content orient="horizontal"
> -             onclick="DownloadsView.onDownloadClick(event);">
> +             onclick="AllDownloadsView.onDownloadClick(event);">

Wouldn't this break the Downloads Panel?

The idea is that DownloadsView would reference different things based on which window loads the code.
Attachment #8960386 - Flags: review?(paolo.mozmail)
Comment on attachment 8960386 [details]
Bug 1443971 - Remove allDownloadsViewOverlay.xul.

https://reviewboard.mozilla.org/r/229170/#review235412

> Wouldn't this break the Downloads Panel?
> 
> The idea is that DownloadsView would reference different things based on which window loads the code.

I misunderstood Marco's comment over in bug 1443901. I've reverted this and changed places.xul to load the scripts in the old order to avoid the redefinition error.
Comment on attachment 8960386 [details]
Bug 1443971 - Remove allDownloadsViewOverlay.xul.

https://reviewboard.mozilla.org/r/229170/#review236374

Great to see another overlay go away!

::: commit-message-bfb7e:2
(Diff revision 2)
> +Bug 1443971 - Remove allDownloadsViewOverlay.xul. r?Paolo
> +This overlay was used in two places: places.xul and

Update the detailed commit message, add an empty line before it.

::: browser/components/downloads/content/downloadsRichListBox.inc.xul:1
(Diff revision 2)
> +<richlistbox flex="1"
> +             seltype="multiple"
> +             id="downloadsRichListBox" context="downloadsContextMenu"
> +             onscroll="return this._placesView.onScroll();"
> +             onkeypress="return this._placesView.onKeyPress(event);"
> +             ondblclick="return this._placesView.onDoubleClick(event);"
> +             oncontextmenu="return this._placesView.onContextMenu(event);"
> +             ondragstart="this._placesView.onDragStart(event);"
> +             ondragover="this._placesView.onDragOver(event);"
> +             ondrop="this._placesView.onDrop(event);"
> +             onfocus="goUpdateDownloadCommands();"
> +             onselect="this._placesView.onSelect();"
> +             onblur="goUpdateDownloadCommands();"/>

It might make sense to use "hg mv --after" on this too, to make clear where the code comes from.
Attachment #8960386 - Flags: review?(paolo.mozmail) → review+
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ef0997b1193
Remove allDownloadsViewOverlay.xul. r=Paolo
https://hg.mozilla.org/mozilla-central/rev/7ef0997b1193
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.