1424672 - panel-header is gone when go to Recently Closed Tabs in panelmultiview

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[magicp]

Attachment: panel-header-is-gone.mp4

Steps to reproduce:
1. Launch Nightly
2. Navigate to Library button > History >  Recently Closed Tabs (or Recently Closed Windows)

Actual Results:
panel-header is gone.

Similar issue of Bookmarks menu has been fixed in bug 1423891. But, this bug is a different STR.

Expected Results:
panel-header should be shown.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5f90362bbc80cf7f9b09e41477c244f292fc9f1c&tochange=6b5a357d277b83349223792361a0fdcc90c15305

Comment 1

[:Gijs (he/him)]

Attachment: Bug 1424672 - stop removing the header in the recently closed places views,

Review commit: https://reviewboard.mozilla.org/r/207182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207182/

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8936466 [details]
Bug 1424672 - stop removing the header in the recently closed places views,

https://reviewboard.mozilla.org/r/207182/#review213062

It still sounds a little bit fragile (in the sense may break easily again in the future if other static content is added), but other ideas look more complex, for example in Places views we insert an invisible element that marks the beginning of the content, and we shouldn't over-engineer for this. I guess for now it's ok, it would be nice to have a more general solution to the problem (custom elements? no idea...).

::: browser/components/places/content/browserPlacesViews.js:243
(Diff revision 1)
>    },
>  
>    clearAllContents(aPopup) {
> -    while (aPopup.firstChild) {
> -      aPopup.firstChild.remove();
> +    let kid = aPopup.firstChild;
> +    while (kid) {
> +      let next = kid.nextSibling;

any reason to not just do:
for (let kid = .firstChild; kid; kid = kid.nextSibling)?

Comment 3

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [::mak] (Away 16 Dec - 2 Jan) from comment #2)
> any reason to not just do:
> for (let kid = .firstChild; kid; kid = kid.nextSibling)?

oh, right, removing the node, nvm.

Comment 4

[:Paolo Amadini]

Comment on attachment 8936466 [details]
Bug 1424672 - stop removing the header in the recently closed places views,

https://reviewboard.mozilla.org/r/207182/#review213152

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4087b735507
stop removing the header in the recently closed places views, r=mak,Paolo

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c4087b735507

Comment 7

[magicp]

This bug fix has verified in the latest Nightly build (20171214220032). Thanks!

Comment 8

[:Gijs (he/him)]

(In reply to magicp from comment #7)
> This bug fix has verified in the latest Nightly build (20171214220032).
> Thanks!

Thank you as always for the prompt and detailed reports of regressions on nightly! It's much appreciated.