Closed Bug 1215418 Opened 6 years ago Closed 6 years ago

[markup-view] HTML editor is incorrectly positioned

Categories

(DevTools :: Inspector, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(firefox44 fixed)

VERIFIED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: sebo, Assigned: jsantell)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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
(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.
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)
Blocks: 1213137
Flags: needinfo?(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
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Flags: needinfo?(jsantell)
Attached patch 1215418-style-regression.patch (obsolete) — Splinter Review
Attachment #8674931 - Flags: review?(bgrinstead)
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?
Attachment #8674931 - Attachment is obsolete: true
Attachment #8674931 - Flags: review?(bgrinstead)
Attachment #8674932 - Flags: review?(bgrinstead)
Comment on attachment 8674932 [details] [diff] [review]
1215418-style-regression.patch

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

LGTM
Attachment #8674932 - Flags: review?(bgrinstead) → review+
https://hg.mozilla.org/mozilla-central/rev/463e8512cb44
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Works. Thanks for the quick fix!

Sebastian
Status: RESOLVED → VERIFIED
Depends on: 1224932
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.