1551578 - [Inactive CSS] Display a warning when text-overflow is used incorrectly

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, enhancement, P3)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Only valid if not a block container element or doesn't clip content:

[
  "text-overflow",
]

[This property] doesn't have an effect because the element is not a block level element or overflow:hidden is set. Try adding a property like display:block or overflow:auto.

Comment 1

[Dhyey Thakore [:dhyey35]]

Hey Mike, I would like to work on this. Can you please guide me about where do I need to make changes ?

Comment 2

[Patrick Brosset <:pbro>]

Great, thanks for your interest.
I've assigned the bug to you.
I'm also marking Mike as the mentor on this bug as he is the best person to help you on this. But let me try to get you on the right path:

The code that needs to be changed is in this file /devtools/server/actors/utils/inactive-property-helper.js.
This contains a sort of rules engine that, given a dom node, a css rule, and a css property name decides whether that property is inactive or not.
The file already contains a number of rules for flex and grid specific css properties (see the VALIDATORS function).
You will need to add a new one specifically for the text-overflow property, so a warning is emitted when it is used on a node that this property does not apply to.

If you look around in this file, you will see a number of helper functions to help you do this. You can check what styles are currently applied to the node for instance with checkStyle.

Now, the second part is the warning itself. It needs to be localized, and therefore is referenced in a localization file: /devtools/client/locales/en-US/tooltips.ftl (this is using the fluent library: https://wiki.mozilla.org/Fluent).

I suggest looking at how other rules do it and doing something similar.

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Dhyey Thakore [:dhyey35] from comment #1)

Hey Mike, I would like to work on this. Can you please guide me about where do I need to make changes?

Please do as Patrick suggested and if you have any issues please needinfo me here again and I will be more than happy to help.

Comment 4

[Dhyey Thakore [:dhyey35]]

Hi Mike, how can I determine if a content is clipped or not ?

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

You just need to check whether the element is a block level element or whether overflow:hidden is set.

Content can be clipped if overflow:hidden is set.

Comment 6

[Patrick Brosset <:pbro>]

Hi Dhyey. I'm wondering if you are still planning on addressing this bug or not. If you just need more time that's fine. No rush.
If you however don't plan on fixing it anymore, please let me know.

Comment 7

[Patrick Brosset <:pbro>]

Unassigning this bug for now. Feel free to comment if you are interested in fixing this bug.

Comment 8

[Pranav pandey]

I want to work on this issue

Comment 9

[Patrick Brosset <:pbro>]

Hi Pranav, thank you for your interest. This bug is available for you to work on, so I'm assigning it to you now.

You will see a lot of information about how to start working on this in comment 2 of this bug.
But before that, since you seem to be new here, please take a look at our contributor guide in order to learn about how to get the code, build and make changes to DevTools: https://docs.firefox-dev.tools/

Comment 10

[Pranav pandey]

Attachment: "Bug 1551578 - displayed a warning when text-overflow is used incorrectly. "

Comment 11

[Patrick Brosset <:pbro>]

Hey Daniel, I'd like to double check one thing with you here before landing this new warning in DevTools. Our plan is to display a warning telling users that text-overflow has no effect on an element if that element is not a block container (display is not any of block or inline-block), and if that element does not have overflow set to hidden.

Is that going to work in all cases, or are we likely to run into false positives (where a warning is shown despite text-overflow actually having effect)? I'm thinking that there might be special cases with grid or flex or tables perhaps.

Comment 12

[Daniel Holbert [:dholbert]]

As long as you're testing computed style when looking for block/inline-block, then I think that mostly makes sense. (This is needed in order to catch the automatic promotion of display:inline elements to display:block when they are floated/abspos/flex-or-grid-items -- e.g. <span style="float:left">)

However, there are some exceptions. The first one is related to tables, as you suspected -- you do need to check for display:table-cell (along with block and inline), because table cells have an anonymous block that wraps their contents, which inherits a bunch of properties, including text-overflow. Here's an example where this property has an effect, with a table-cell:

data:text/html,<span style="display:table-cell;max-width:50px;overflow:hidden;text-overflow:ellipsis">abcdefghijklmnop

This comes into play via this ua.css style rule, BTW (which is where the style inherits from the cell to its anonymous block):
https://searchfox.org/mozilla-central/rev/7e92a667e3829831c31e8d46aefe7ef67ad5be1c/layout/style/res/ua.css#160

(Grid & flex anonymous wrappers do not inherit text-overflow so this isn't an issue for grid/flex AFAICT).

I don't think any of the other text-overflow: inherit declarations in that file (ua.css) are places that would be problematic with your described logic, which is good news. (I wonder a bit about the -moz-block-inside-inline-wrapper one, but I played around & couldn't come up with a way to make text-overflow matter on a display:inline element involved in a block-in-inline split.)

But you do also need to check form controls, many of which also have anonymous block wrappers inside of them, even if they're display:inline (and they inherit text-overflow and overflow into those wrappers). I'm thinking fieldset and button in particular, but really all of the elements listed in forms.css above this style rule with text-overflow:inherit:
https://searchfox.org/mozilla-central/rev/7e92a667e3829831c31e8d46aefe7ef67ad5be1c/layout/style/res/forms.css#769

Some examples that I tested where text-overflow matters for form elements (even if they're display:inline) (note that I'm manually specifying display:inline here, just for clarity/good-measure, even though in some cases it's already the default):

data:text/html,<button style="display:inline;max-width:100px;overflow:hidden;text-overflow:ellipsis">abcdefghijklmnopqrstuv

data:text/html,<fieldset style="display:inline;min-width:0;max-width:50px;overflow:hidden;text-overflow:ellipsis">abcdefghijklmnopqrstuv

data:text/html,<input style="display:inline;min-width:0;max-width:100px;overflow:hidden;text-overflow:ellipsis" value="abcdefghijklmnop">

data:text/html,<select style="display:inline;max-width:100px;overflow:hidden;text-overflow:ellipsis"><option>abcdefghijklmnopqrstuv

As an extra sanity-check, I'll punt your needinfo to mats, since he worked on text-overflow:ellipsis and he may know if there's any other exceptions (besides these ones) that your described logic wouldn't cover.

Comment 13

[Daniel Holbert [:dholbert]]

Also the fieldset <legend> child:

data:text/html,<fieldset><legend style="display:inline;max-width:50px;overflow:hidden;text-overflow:ellipsis">abcdefghijklmnopqrstuv

Comment 14

[Mats Palmgren (inactive)]

(commented inline in the patch)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4222d56ec9d
"Bug 1551578 - displayed a warning when text-overflow is used incorrectly.  " r=pbro

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

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