1369659 - The lock and header url in the site identity subview are not on the same line

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[Johann Hofmann [:johannh]]

See mozscreenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=edffa38ec0c515198f360f23d286290cb5081996&newProject=mozilla-central&newRev=cac2fd43de81d44b36396704220996b0c4a39f1a&filter=subview

Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=edffa38ec0c515198f360f23d286290cb5081996&tochange=cac2fd43de81d44b36396704220996b0c4a39f1a

Mike mentioned he might know what caused this or how to fix it. Thanks for jumping in!

Comment 1

[Mike de Boer [:mikedeboer]]

Gijs, why was the change from

`panelview:not([title]) > .panel-header` to `panelview[mainView] > .panel-header` necessary again? This rule change in browser.css causes the header to be shown in all panelmultiviews, not just the photon one. :(

Comment 2

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1369659 - Make sure to hide the panelview header in non-photon panels, because it breaks their layout.

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

Comment 3

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #1)
> Gijs, why was the change from
> 
> `panelview:not([title]) > .panel-header` to `panelview[mainView] >
> .panel-header` necessary again? This rule change in browser.css causes the
> header to be shown in all panelmultiviews, not just the photon one. :(

Because otherwise the title is visible when a view with a title (such as the library view) is displayed as a main view (ie when displayed using the library button in the toolbar).

Comment 4

[:Gijs (he/him)]

Comment on attachment 8873766 [details]
Bug 1369659 - Make sure to hide the panelview header in non-photon panels, because it breaks their layout.

https://reviewboard.mozilla.org/r/145178/#review149146

Tentative r=me... can we also fix this by re-adding a :not([title]) selector in addition to the panelview[mainview] one (checking carefully that the library panel still works correctly when first opened as a subview in the main photon panel (which will cause the title attribute to get set based on its anchor there), and then as a 'standalone' panel)? If so, I would prefer that, because I imagine we want to eventually switch all the panelmultiviews to photon ones, and then get rid of the 'photon' in the name, at which point we'd have to fix this again...

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8873766 [details]
Bug 1369659 - Make sure to hide the panelview header in non-photon panels, because it breaks their layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/145178/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb5fc68d7f71
Make sure to hide the panelview header in non-photon panels, because it breaks their layout. r=Gijs

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 8

[Johann Hofmann [:johannh]]

Looks fixed, thank you!

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=c67d811399a1408dd4c77a92584ecf480e3c972b&newProject=mozilla-central&newRev=43039280fe464869428f03b047bb7c762784f44b&filter=controlCenter