Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4657 closed Bug (fixed)

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

Reported by: Frederico Caldeira Knabben Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.4.1
Component: Core : Styles Version: 3.0
Keywords: IBM Opera Cc: Satya Minnekanti, Damian, joek, Hallvord R. M. Steen (Opera Software)

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 Frederico Caldeira Knabben 7 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

#4807 has been marked as DUP.

comment:3 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Cc: Satya Minnekanti Damian joek added
Keywords: IBM added

#5331 has been marked as dup

comment:4 Changed 7 years ago by Frederico Caldeira Knabben

Cc: Hallvord R. M. Steen (Opera Software) added

comment:5 Changed 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Frederico Caldeira Knabben

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 7 years ago by Hallvord R. M. Steen (Opera Software)

OK thanks, I will keep looking at it

comment:8 Changed 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Krzysztof Studnik

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 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Frederico Caldeira Knabben

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 7 years ago by Krzysztof Studnik

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 7 years ago by Frederico Caldeira Knabben

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 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Frederico Caldeira Knabben

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 Changed 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Hallvord R. M. Steen (Opera Software)

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

Changed 7 years ago by Frederico Caldeira Knabben

Attachment: 4657.patch added

comment:18 in reply to:  16 Changed 7 years ago by Frederico Caldeira Knabben

Owner: set to Frederico Caldeira Knabben
Status: confirmedreview

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 7 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1

comment:20 Changed 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

comment:22 Changed 7 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: review_passedclosed

Fixed with [5883].

comment:23 Changed 7 years ago by Satya Minnekanti

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 Changed 7 years ago by Hallvord R. M. Steen (Opera Software)

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 7 years ago by Satya Minnekanti

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 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy