Background color height is incorrect on large text

[jhchen]

1. Select a word
2. Make the font size large
3. Add background color

Background color does not fill the entire height. Seems to be because the span controlling the size does not expand its parent (which controls the color). Applying the formatting in reverse (color then size) does not have this issue.

[amerikan]

Unless I have misunderstood the issue, the issue has been fixed (6 months old) or I am failing to follow the steps you gave correctly, I don't see this happening, at least not in Firefox 29.0/OS X.10.9.2

[jhchen]

You are right the step I gave works now but this is unfortunately just a side effect of some internal ordering changes. The reproduction steps are now reversed:

1. Select word
2. Add background color
3. Make font size large

[amerikan]

Ok now I can reproduce.

[krishvs]

Is there any specific reason why fontsize range is being converted to pixels?

[jhchen]

There could be some flexibility here. What would be your recommendation @krishvs ?

[krishvs]

I dont think this can be fixed using only css and the execCommand or range.As a custom undo manager exists, dom manipulation is the way to go.Have started playing around with the code and will submit a pull request if i am able to fix it.

[amerikan]

When the font size is applied to the outer element, the background color looks as it should, however, the other way around is when the issue arises. I'm not familiar enough with coffeescript yet, but as @krishvs mentioned you'll have to manipulate the DOM to always wrap the text with the background span first and font size second.

Looks OK:

<span style="font-size: 32px;">
    <span style="background-color: rgb(255, 194, 102);">lt for the modern we</span>
</span>

Looks BAD

<span style="background-color: rgb(102, 163, 224);">
    <span style="font-size: 32px;">e case. Some built in features includ</span>
</span>

Result

[jhchen]

Still an issue for IE: ef93721

[jhchen]

Sorry had to revert this fix. Turns out it's not a good solution for other browsers either.

[EricGrange]

A possible fix is to specify "background-color: inherit" in the font-size span

[jhchen]

Excellent idea! Thanks!

[EricGrange]

The fix you committed only works in the editor, and not when the html is displayed afterwards.
The "background-color: inherit" should be applied alongside the font-size in the span's inline style, not in the CSS

[jhchen]

What do you mean when the html is displayed afterwards? The only difference between an inline style and css style is precedence.

[EricGrange]

By afterward I mean "once it has been edited", the purpose of the editor being to edit rich text as html, this html is also meant to be displayed after its edition, without no editor in sight.

The main difference between inline & css style is not precedence, it is that inline style is part of the html, so when you display the html, inline style is guaranteed to be there, while stylesheet css is not.

It would be different if the editor was doing its editing based on css classes rather than inline styles, then having an external stylesheet to go alongside the html would be expected. But given that the editor edits inline styles, IMHO it should maintain coherent inline styles, rather than require a specific css stylesheet for the html to be displayed "as edited".

[jhchen]

There are already several rules in the base Quill stylesheet that would have to be replicated to maintain visual correspondence with the editor. It's also not sufficient to just add the inherit rule to spans--it'd have to be added to all inline styles (otherwise color -> bold -> size would still trigger the bug) which is much messier markup. Fwiw Quill is moving away from inline styles by default in some future release.