Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#4657 closed Bug (fixed)

Opera: It's not possible to apply styles on collapsed selections

Reported by: fredck Owned by: fredck
Priority: Normal Milestone: CKEditor 3.4.1
Component: Core : Styles Version: 3.0
Keywords: IBM Opera Cc: satya, damo, joek, hallvord@…

Description

As of Opera 10, it's not anymore possible to apply styles on collapsed selections.

TC:

  1. Click in the editing area to have a collapsed selection (blinking caret).
  2. Hit CTRL+B to apply the Bold style.
  3. Type some text.

The typed text should be bold.

Confirmed with Opera 10.10 build 1874.

Attachments (1)

4657.patch (794 bytes) - added by fredck 6 years ago.

Download all attachments as: .zip

Change History (26)

comment:2 Changed 7 years ago by fredck

#4807 has been marked as DUP.

comment:3 Changed 7 years ago by alfonsoml

  • Cc satya damo joek added
  • Keywords IBM added

#5331 has been marked as dup

comment:4 Changed 6 years ago by fredck

  • Cc hallvord@… added

comment:5 follow-up: Changed 6 years ago by hallvord@…

This seems to work fine on this very basic test case:

data:text/html,<body contenteditable="true">

I also note that Chrome seems to behave like Opera when testing this on ckeditor.com/demo so my conclusion is that it's a bug in CKEditor. I tried to figure out exactly what goes wrong, but the code is quite complex and I don't have time to get through it right now.

comment:6 in reply to: ↑ 5 Changed 6 years ago by fredck

Replying to hallvord@…:

The main problem here is that this thing used to work with previous Opera versions. It surely worked with the 9.50, and maybe even with the 10.00. Then, suddenly, it stopped working. So, something happened at Opera.

Regarding Chrome, this is old issue we have with WebKit, and it's a browser bug. We have #1272 for it.

I thought Opera was suffering from the same WebKit issue, but I found out that the original webkit tc, linked to its relative webkit ticket, works well for Opera. So, it's something different.

We'll do some investigation here, cause we've made changes to the style system for version 3.4, but I think it's work to at least know which Opera build introduced the problem, and eventually why.

comment:7 Changed 6 years ago by hallvord@…

OK thanks, I will keep looking at it

comment:8 Changed 6 years ago by hallvord@…

It works for me after commenting out the browser sniffing here:

if(z.collapsed/* &&b.gecko&&b.version<10900 */&&B.type==1&&!B.getChildCount())B.appendText();

comment:9 Changed 6 years ago by krst

Bold button does not activate, after clicking on it. Bold style is applied only to selected text

Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk

comment:10 Changed 6 years ago by hallvord@…

krst: it is a bug in CKEditor's browser sniffing. Or rather, web browsers are trying harder to be compatible and changing to me more similar - so CKEditor should sniff less :-D

comment:11 Changed 6 years ago by fredck

hallvord: you, better than anyone else, knows that the compatibility browsers aim to have is currently utopia. Sometimes we need to sniff, to workaround browsers bugs an limitations.

Let me point you to the part of sniffing you're happily ;) commenting out:
source:/CKEditor/trunk/_source/plugins/selection/plugin.js@5812#L1103

Basically, you're saying that we should not sniff it there at all, having all browsers running that piece of weird code which is currently exclusive to our (not so) good and old Firefox 2! The answer for it is certainly "No!" :P

A possible fix for this ticket at this point would be adding Opera to that sniffing train. It means putting Opera and Firefox 2 hand to hand, and it doesn't sound like a good solution for me, specially considering the future and your "trying hard to be compatible" words. I would instead prefer declaring FF2 incompatibility, removing those lines, forgetting them forever.

So, to conclude: what path do you think we should take in this ticket?

comment:12 Changed 6 years ago by krst

  • Keywords Webkit added

Confirmed also in Chrome 7, also Safari So it is also problem with webkit engine. Is it possible to fix it in CKEditor?

comment:13 Changed 6 years ago by fredck

  • Keywords Webkit removed

#1272 is the ticket for WebKit, which always had this issue. Opera introduced it later.

We're using two different tickets in this case, because the WebKit issue is a CantFix, so it's better to keep track of their developments separately (we'll hardly be able to get it fixed for both browsers at the same time).

comment:14 Changed 6 years ago by hallvord@…

Fred, I'm always happy to comment out browser sniffing ;) though since you mention it I guess bug compatibility with Firefox 2 isn't exactly a design goal these days. The link to the full and commented source code helps a lot, thanks. I'll have a third look..

comment:15 Changed 6 years ago by fredck

Hallvord, as commented earlier, this issue has been introduced at some point in an old Opera release (used to work with 9.50). Maybe you have something like our revision web site there, which would help you to easily find the exact Opera build that introduced the problem. Just an idea.

comment:16 follow-up: Changed 6 years ago by hallvord@…

regression since Opera 10, and I guess the cause was CORE-8657 (aka #1271 here) :-p

Fred, your WebKit TC linked above has a workaround that inserts an empty text node, so it appears to work in Opera. However, I've done some extensive stepping through the relevant CKEditor code now, and I don't see this workaround anywhere. Hence it fails (and while Opera 9.50 did not actually need that workaround, 10.x does again :-/).

I would suggest changing range.js to insert a new block below the one with the comment "Fixing invalid range start inside dtd empty elements":

// Fixing invalid range start inside dtd empty elements.
if( startNode.type == CKEDITOR.NODE_ELEMENT
	&& CKEDITOR.dtd.$empty[ startNode.getName() ] )
	startNode = startNode.getParent(), startOffset = startNode.getIndex();
// if the element we want to place the range in is entirely empty, the cursor has nowhere to go in some browsers (#4657)
if( startNode.type == CKEDITOR.NODE_ELEMENT
	&& startNode.$.childNodes.length==0 )
	startNode.$.appendChild(document.createTextNode(''));

comment:17 Changed 6 years ago by hallvord@…

(On Opera's side the underlying issue is tracked as CORE-5953)

Changed 6 years ago by fredck

comment:18 in reply to: ↑ 16 Changed 6 years ago by fredck

  • Owner set to fredck
  • Status changed from confirmed to review

Replying to hallvord@…:

However, I've done some extensive stepping through the relevant CKEditor code now, and I don't see this workaround anywhere. Hence it fails (and while Opera 9.50 did not actually need that workaround, 10.x does again :-/).

That's the point. When we started working on CKEditor 3.0, we had Opera 9.50 and we happily noticed that this was not a problem anymore with it. We ended up leaving that hack to Firefox 2 only.

The attached patch simply activate the FF2 fix on Opera also. Well, if I would be Opera, I'll not be happy with it :P

comment:19 Changed 6 years ago by fredck

  • Milestone set to CKEditor 3.4.1

comment:20 Changed 6 years ago by hallvord@…

Well, I'm not happy with it Fred - nagging the right people about CORE-5953 ;).

I've also reported two follow-up bugs on issues that the empty text node can cause. That said, I don't find the workaround too horrendous - if you think that the cursor needs "somewhere" to be, and throw in an empty text node to give it "somewhere" to go it sort of makes sense. We will fix it, but thanks for the workaround while we're massaging the cursor code.

comment:21 Changed 6 years ago by Saare

  • Status changed from review to review_passed

comment:22 Changed 6 years ago by fredck

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [5883].

comment:23 Changed 6 years ago by satya

This is not properly fixed. http://dev.ckeditor.com/ticket/5331 has marked as a duplicate of this ticket.

Issues that i am still seeing are.

1) with out keeping focus in Editor body, if we select the options in more than 1 drop down list(Styles/Paragraph format,Font Name,Font Size,Text Colour & Background Colour) at a time focus is going out of Editor body.

2) By keeping focus in Editor body, if we select an option in one of drop down lists(Styles/Paragraph format,Font Name,Font Size,Text Colour & Background Colour) focus is going out of Editor body.

3) when we select a Font Name( ex: Verdana) & when we type the text,Correct Font Name is applied to the typed text, but the option is not shown in Font Names drop down list.

comment:24 follow-up: Changed 6 years ago by hallvord@…

satya: I don't see those issues in Opera 11 / CKEditor 3.5 (and if you still see them, please report separate bugs, I don't think they are related to this one)

comment:25 in reply to: ↑ 24 Changed 6 years ago by satya

Replying to hallvord@…:

satya: I don't see those issues in Opera 11 / CKEditor 3.5 (and if you still see them, please report separate bugs, I don't think they are related to this one)

All of the above issues are fixed in opera 11 / CKEditor 3.5

Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy