Found in #3153260: Standardize Olivero's focus states across all non-form controls.

Olivero has an unneeded CSS class where it styles text if CKEditor is in the footer region.

It's extremely unlikely a user will add the CKEditor into the footer, so this class can be removed.

CommentFileSizeAuthor
#15 interdiff_8-15.txt1.77 KBmike.roman
#12 3200599-9.patch2.07 KBmike.roman
#10 3200599-patch.png127.2 KBmeghasharma
#8 3200599-8.patch1.45 KBimalabya
#7 3200599_07.patch1.53 KBhinal05
#5 class-removed-3200599-4.patch6.92 KBbhumikavarshney
#4 class-removed-3200599-3.patch1.53 KBgauravvvv
#2 class-removed-3200599-2.patch1.53 KBgauravvvv
footer-ckeditable.png223.79 KBmherchel

Comments

mherchel created an issue. See original summary.

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new class-removed-3200599-2.patch1.53 KB

I have removed the class, Please review.

gauravvvv’s picture

gauravvvv commented
Status: Active » Needs review
gauravvvv’s picture

gauravvvv commented
StatusFileSize
new class-removed-3200599-3.patch1.53 KB
bhumikavarshney’s picture

bhumikavarshney commented
StatusFileSize
new class-removed-3200599-4.patch6.92 KB
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

the testing suite is not able to apply the attached patches.

hinal05’s picture

hinal05 commented
Status: Needs work » Needs review
StatusFileSize
new 3200599_07.patch1.53 KB

Please review the patch.

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
StatusFileSize
new 3200599-8.patch1.45 KB

Re-rolled the patch.

Madhu kumar’s picture

Madhu kumar commented

Patch #8 applied cleanly and it is working well.

meghasharma’s picture

meghasharma commented
StatusFileSize
new 3200599-patch.png127.2 KB

Patch #8 applied cleanly and it is working well.
Looks good to me
screenshot for reference.
No need for cke_editable class in footer CSS issue RESOLVED
The cke_editable class has removed from text-content.css file.
RTBC

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work 
+++ b/core/themes/olivero/css/components/text-content.pcss.css
@@ -142,8 +142,7 @@
+  & .text-content {

This works, but should be simplified to .site-footer .text-content { } since there is now only one descendent selector.

@meghasharma, screenshots of code do not help, as the testing infrastructure will let us know if the patch does not apply.

mike.roman’s picture

mike.roman commented
StatusFileSize
new 3200599-9.patch2.07 KB

I made changes to imalabya's "-8" patch based on mherchel's recommendation. I'll try to get an interdiff uploaded later. #midcamp2021

mike.roman’s picture

mike.roman commented
Status: Needs work » Needs review
mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue tags: +midcamp2021

Adding tag.

mike.roman’s picture

mike.roman commented
StatusFileSize
new interdiff_8-15.txt1.77 KB

Following up on my comment #12, this is the interdiff file.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Reviewed & tested by the community

#14 looks great. Thanks!

Status: Reviewed & tested by the community » Needs work

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

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Reviewed & tested by the community

Test failure was unrelated. Setting back to RTBC

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

This looks good as well. I believe that the .site-footer .cke_editable selector would never be applied because .cke_editable is used inside the iframe content. I will get back to this in a couple of days after the security release window.

  • lauriii committed 569ac38 on 9.2.x 
    Issue #3200599 by Gauravmahlawat, mike.roman, BhumikaVarshney, hinal05,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed 569ac38 and pushed to 9.2.x. Thanks!

Status: Fixed » Closed (fixed)

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