New issue
Advanced search Search tips

Issue 739403 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 733154
issue 739139



Sign in to add a comment

Prepare EditingStyle for CSSPrimitiveValue type in font-style and font-weight

Project Member Reported by drott@chromium.org, Jul 5 2017

Issue description

EditingStyle does certain equivalence checks to convert between <b> tags and <i> tags and discard CSSValueNormal styles when formatting text. 

As far as I understand the code so far, this works based on a list of HTMLElementEquivalent objects which define the relationship between tags and their formatting effect. 

In https://chromium-review.googlesource.com/c/552556/ and  issue 739139  and  issue 733154  I am working on new parsing mechanisms for reading https://drafts.csswg.org/css-fonts-4/#font-style-prop and https://drafts.csswg.org/css-fonts-4/#font-weight-prop as CSSPrimitiveValue of type number, instead of CSSIdentifier value.

After doing that, lots of editing tests fail. 

yosin@, what do you recommend here? Would it make sense to upgrade HTMLElementEquivalent to support additional CSSValue types, not only CSSIdentifierValue? 

Could you help explain how I should modify EditingStyle? 
 

Comment 1 by yosin@chromium.org, Jul 6 2017

Components: Blink>Editing
Owner: drott@chromium.org
Status: Assigned (was: Untriaged)
The purposes of HTMLElementEquvalent is 
- reducing size of serialization to avoid
redundant inline style, e.g. adding "font-weight: bold" for B is redundant for "<div style="font-weight: bold">abc <b>def</b> ghi</div>".
- style detection for UI command "bold", "italic", etc. For element with "font-weight: 1000", we want to say this element is already bold.

So, if you keep in the mind of them, it is OK to adopt EditingStyle for new font features.

Comment 2 by drott@chromium.org, Jul 8 2017

Thanks for your help and suggestions. I think I found the spot in EditingStyle where I can fix FontWeightIsBold() detection and for EditingCommand Format Bold it seems to work now. But there is still a problem with Italic. ToggleItalic and EditingCommand format italic do not seem to recognize italic somewhow and a lot of pasteboard tests fail because there is an unnecessary "font-style: normal;" inserted. 

Is there some kind of detection for initial CSS values that might fail here? Any other suggestions are highly welcome.

Comment 3 by yosin@chromium.org, Jul 10 2017

It seems this is caused by <angle> optional parameter of "oblique <angle>?".

Do we get |int| value for "font-style" of  GetIdentifierValue(style, CSSPropertyFontStyle) in  StyleChange::ExtractTextStyles()?



Comment 4 by drott@chromium.org, Jul 11 2017

Status: WontFix (was: Assigned)
Thanks for your help. I'll close this for now, I think I have a way to address this issue.

Sign in to add a comment