Brendan Dahl [:bdahl] Assignee
	Description • 6 years ago

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.

	:Paolo Amadini
	Comment 2 • 6 years ago
mozreview-review

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.

	Brendan Dahl [:bdahl] Assignee
	Comment 4 • 6 years ago
mozreview-review-reply

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.

	:Paolo Amadini
	Comment 5 • 6 years ago
mozreview-review

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.

	Pulsebot
	Comment 7 • 6 years ago

Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ef0997b1193
Remove allDownloadsViewOverlay.xul. r=Paolo

	Stefan Hindli [:stefan_hindli]
	Comment 8 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/7ef0997b1193