Problem/Motivation

Claro's elements.css contains this:

ol {
  margin: 0.25em 0 0.25em 2em; /* LTR */
  padding: 0;
  list-style-type: decimal;
}

Steps to reproduce

This interferes with https://developer.mozilla.org/en-US/docs/Web/HTML/Element/ol#attr-type — it means that even if you create <ol type="A"><li>first</li><li>second</li></ol>, in Claro this markup will be rendered with decimals instead of capital letters.

This is especially bad when using a text editor that does not use iframes. CKEditor 4 rendered its markup in an iframe. CKEditor 5 does not.

Discovered in #3274651: Impossible to enable <ol type> or <ul type> with GHS: switch to List's successor, DocumentList.

Proposed resolution

Omit list-style-type: decimal;, or change specific selector specificity.

Remaining tasks

TBD

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

None.

CommentFileSizeAuthor
#16 3282598-nr-bot.txt90 bytesneeds-review-queue-bot
#9 3282598-9.patch766 bytespradhumanjain2311
#7 3282598-7.patch363 byteslalitware
#7 interdiff_2-7.txt353 byteslalitware
#2 3282598.patch382 bytesamit.mall

Issue fork drupal-3282598

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Wim Leers created an issue. See original summary.

amit.mall’s picture

amit.mall commented
StatusFileSize
new 3282598.patch382 bytes

valid issue.
Removed from core/themes/claro/css/base/elements.css

amit.mall opened merge request !2336

amit.mall closed merge request !2336

amit.mall’s picture

amit.mall commented
Status: Active » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 10.0.x-dev » 10.1.x-dev
Status: Needs review » Needs work
Issue tags: +Needs reroll

patch needs to be cleaned up to pass linting (remove extra space at the end of the line)

lalitware’s picture

lalitware commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new interdiff_2-7.txt353 bytes
new 3282598-7.patch363 bytes

Removed extra space after padding property.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work

almost there! The change needs to happen in the elements.pcss.css file.

Before making your patch you need to run yarn build to generate the elements.css file automatically. See https://www.drupal.org/about/core/policies/core-change-policies/frontend... for details :)

pradhumanjain2311’s picture

pradhumanjain2311 commented
Status: Needs work » Needs review
StatusFileSize
new 3282598-9.patch766 bytes

Try to fix patch #7 as @nod_ suggested.
please review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Looks like change requested in #8 was addressed. And this seems like this could be a problem for ckeditor5

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 3282598-9.patch, failed testing. View results

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Gauravvvv made their first commit to this issue’s fork.

Gauravvvv opened merge request !7755

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review

Created MR for same, as the patch #9, no longer applies.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3282598-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Gauravvvv changed the visibility of the branch 3282598-decimal-list to hidden.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review

Gauravvvv opened merge request !7838

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Change appears simple enough.

nod_ closed merge request !7838

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 91e3259 and pushed to 11.x. Thanks!

nod_’s picture

nod_
Primary language French
Location Lille
commented

  • nod_ committed fb454de1 on 10.3.x 
    Issue #3282598 by Gauravvvv, amit.mall, lalitware, pradhumanjain2311,...

  • nod_ committed 584e781e on 10.4.x 
    Issue #3282598 by Gauravvvv, amit.mall, lalitware, pradhumanjain2311,...

  • nod_ committed 919a4889 on 11.0.x 
    Issue #3282598 by Gauravvvv, amit.mall, lalitware, pradhumanjain2311,...

  • nod_ committed 91e32596 on 11.x 
    Issue #3282598 by Gauravvvv, amit.mall, lalitware, pradhumanjain2311,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.