Fix text wrap of boxmodel-editable

VERIFIED FIXED in Firefox 60

Status

defect
P3
normal
VERIFIED FIXED
a year ago
10 months ago

People

(Reporter: magicp.jp, Assigned: abhinav.koppula)

Tracking

({good-first-bug})

Trunk
Firefox 60

Firefox Tracking Flags

(firefox60 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Steps to reproduce:
1. Launch Nightly
2. Go to https://bugzilla.mozilla.org
3. Open Inspector
4. Inspect #header-tools-menu
5. Switch sidebar tab to "Computed"

Actual results:
Text wrap and overlap with margin-top.

Expected results:
No wrap.
Thanks for filing and including the screenshot.

This looks like a CSS change that should be relatively simple to make.
I think we should make all of the labels inside this box-model view have the following property: white-space: nowrap;
This can be fixed in the following file: /devtools/client/themes/boxmodel.css
My hunch is that we should apply this too all .boxmodel-editable elements, but the person working on this bug should check that this doesn't have unwanted impacts.

I'll mark this as a good-first-bug because it shouldn't be hard to fix.
If you're new to Firefox and DevTools development, please read our docs to get started. And feel free to ask questions and claim this bug in comments here directly. I'd be happy to help you get this fixed.

Docs: http://docs.firefox-dev.tools/
Keywords: good-first-bug
Priority: -- → P3
Comment hidden (mozreview-request)
(Assignee)

Comment 3

a year ago
Hi Patrick,
I've taken a shot at this issue.
Can you please review it?

Comment 4

a year ago
mozreview-review
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222054

Thank you! This seems to work fine. I think you can simplify the commit even more (see comment below).
Are you able to attach a new commit to fix this?

::: devtools/client/themes/boxmodel.css:311
(Diff revision 1)
> +  white-space: nowrap;
>  }
>  
>  .boxmodel-editable:hover {
>    border-bottom-color: hsl(0, 0%, 50%);
> +  white-space: nowrap;

Is this really needed? If this property is already defined on `.boxmodel-editable`, I don't think you need to define it too on `:hover`.
Attachment #8945829 - Flags: review?(pbrosset)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

a year ago
mozreview-review-reply
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222054

Yes, it works well even without defining in on :hover. I think that is redundant.
I've updated my patch.

Comment 7

a year ago
mozreview-review
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222060

Great. Thanks for being so quick.
Attachment #8945829 - Flags: review?(pbrosset) → review+
(Assignee)

Comment 9

a year ago
Hey Patrick,
There are some failures in TRY but I don't think they are related to this fix.
Can you check once?
Flags: needinfo?(pbrosset)
(Assignee)

Updated

a year ago
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Yup, looks good to me.
Flags: needinfo?(pbrosset)

Comment 11

a year ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c2881a440f7
Fix text wrap of boxmodel-editable; r=pbro

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4c2881a440f7
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
(Reporter)

Comment 13

a year ago
Thanks Abhinav! I have verified in the latest Nightly build (20180201220225) on Win10, Ubuntu 17.10 and macOS 10.13.
On second thought, maybe we could uplift this?
Status: RESOLVED → VERIFIED

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.