1215418 - [markup-view] HTML editor is incorrectly positioned

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[Sebastian Zartner [:sebo]]

When right-clicking an element within the Inspector and choosing 'Edit As HTML' the editor is now placed far to the right.

This happens since today's Nightly update. So the regression range should be https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=00f555ed6608&tochange=d374d16cbb25.

Sebastian

Comment 1

[Brian Grinstead [:bgrins]]

(In reply to Sebastian Zartner [:sebo] from comment #0)
> When right-clicking an element within the Inspector and choosing 'Edit As
> HTML' the editor is now placed far to the right.
> 
> This happens since today's Nightly update. So the regression range should be
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=00f555ed6608&tochange=d374d16cbb25.
> 
> Sebastian

Thanks for the report.  This is weird, I've checked to make sure it's not Bug 1214663 or Bug 1189464 - we need to get a regression window for this.

Comment 2

[Brian Grinstead [:bgrins]]

Hmm looks like there is a .theme-body rule in common.css with margin: 0 that's overriding the margin on the html editor.

Jordan, looks like this is a regression due to Bug 1213137.  I'm not sure that we should define any positioning rules for .theme-body, right now it's used only for text/background styling and it's used pretty widely.  Additionally, this kind of rule should live in the themes and not common.css since that's loaded in browser.xul.

I'd suggest instead that you could handle this in the light and dark themes on the `body` selector, or this could be handled per-tool (right now we only have a couple of frames that are HTML and I think each does it's own positioning reset)

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Good catch, grabbing this -- testing a few views this didn't seem to make a change, I wonder if this should only be applied for HTML documents, as it seems necessary -- either way, can handle in another bug, working on a quick patch now for this

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1215418-style-regression.patch

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8674931 [details] [diff] [review]
1215418-style-regression.patch

Review of attachment 8674931 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/themes/light-theme.css
@@ +4,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  @import url(variables.css);
>  
> +.theme-body.html {

What about `body.theme-body` and get rid of the html attr?

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1215418-style-regression.patch

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8674932 [details] [diff] [review]
1215418-style-regression.patch

Review of attachment 8674932 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/463e8512cb44

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/463e8512cb44

Comment 10

[Sebastian Zartner [:sebo]]

Works. Thanks for the quick fix!

Sebastian