Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Border style reducer does not return all possible shorthands for CSS properties #10399

Closed
maxbarnas opened this issue Aug 23, 2021 · 7 comments 路 Fixed by #15868
Closed

Border style reducer does not return all possible shorthands for CSS properties #10399

maxbarnas opened this issue Aug 23, 2021 · 7 comments 路 Fixed by #15868
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@maxbarnas
Copy link
Contributor

馃摑 Provide detailed reproduction steps (if any)

Assuming this GHS pattern:

styles: {
  'border-left': /.*/
}

and markup like below:

<p style="border-left: 1px solid;">
  foobar
</p>

There is a bug where border style reducer returns only style properties like border-left-width: 1px and border-left-style: solid, but not border-left: 1px solid which prevents Matcher from correctly matching pattern to the style of the element.

The source of this behavior can be found here: https://github.com/ckeditor/ckeditor5/blob/e6cd6e4/packages/ckeditor5-engine/src/view/styles/border.js#L331-L362 .

This is a follow-up to this comment.


If you'd like to see this fixed sooner, add a 馃憤 reaction to this post.

@maxbarnas maxbarnas added type:bug This issue reports a buggy (incorrect) behavior. package:engine domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. package:html-support labels Aug 23, 2021
@pomek
Copy link
Member

pomek commented Aug 23, 2021

border-left: 1px solid

This definition is not valid (for me). If you define an element with such style, a browser will use #000 color as default instead of the specified in CSS for an element, so it ends with wrong styles, e.g. in table cells (see: #9790 (comment))

See the attached image:

image

Border-color: #000 (not specified anywhere, should be #ff0)

vs.

image

Border-color: #ff0 (as the specified for p).

The short notation must contain all three definitions: width, style, and color.

@maxbarnas
Copy link
Contributor Author

maxbarnas commented Aug 23, 2021

When stating that it's valid syntax, I was referring to https://drafts.csswg.org/css-backgrounds-3/#border-shorthands or this https://www.w3.org/TR/CSS2/box.html#border-shorthand-properties .

@pomek
Copy link
Member

pomek commented Aug 23, 2021

I read the spec, and now I see that these values are not obligatory.

So there must be a reason why browsers inherit the initial value instead of the last specified in a tree.

@maxbarnas
Copy link
Contributor Author

What the style reducers do now might be fine for our needs and they might be not in line with the spec. It's just that for now, Matcher is incorrectly working because of the missing style. We might as well modify the Matcher instead of reducers - I just wanted to point out that there's a problem.

@108signals
Copy link

@maxbarnas @pomek wanted to see if there are any updates on this issue? Unfortunately our users have flagged this a few times since we first reported it here

cc @magda-chrzescian @mlewand

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Jan 13, 2022
@108signals
Copy link

@maxbarnas @pomek wanted to see if are any updates on this issue? We are getting users flagging this issue on a monthly basis.

cc @magda-chrzescian @mlewand @mdabkowska88

@katypodbielski
Copy link

Hi CKE Team, I'm writing to see if there are any updates on this issue? Our users continue to flag and follow-up about this ticket. Resolving would improve the experience of using tables. Thank you

@maxbarnas @pomek @magda-chrzescian @mdabkowska88 @mlewand

niegowski added a commit that referenced this issue Feb 22, 2024
Fix (html-support): Background color style should be properly preserved by GHS while the `FontBackgroundColor` plugin is enabled. It should be able to preserve a partly defined style. Closes #15757. Closes #10399.
@CKEditorBot CKEditorBot added this to the iteration 72 milestone Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants