Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#1271 closed Bug (worksforme)

Opera: Small space is appended when applying styles to collapsed selections

Reported by: Frederico Caldeira Knabben Owned by: Hallvord R. M. Steen (Opera Software)
Priority: Must have (possibly next milestone) Milestone: Opera Compatibility
Component: Core : Styles Version: SVN (FCKeditor) - Retired
Keywords: Cc: Hallvord R. M. Steen (Opera Software)

Description

This bug is a follow up to #253. This specific feature has been introduced for that ticket, but Opera presents a strange behavior.

Steps to Reproduce

  1. Load sample01.html;
  2. Place the cursor in the middle of the word "some". For example, after the "o";
  3. Press CTRL+B several times. The toolbar will behave correctly to the command, but note that a small space is added for each time it get pressed.

Analyzing the DOM after CTRL+B, all we have is "so<b></b>me", with the selection inside <b>. That was our intention, but the small space get rendered for the empty <b>.

The code which applies the style can be found in the "_ApplyInlineStyle" function in fckstyle.js. It can be condensed to the following code, which is the only way we have found to make it work with Opera:

// Here the <b> tag is build.
var collapsedElement = this.BuildElement( range.Window.document ) ;
collapsedElement.innerHTML = '&nbsp;' ;
range.InsertNode( collapsedElement ) ;
range.MoveToNodeContents( collapsedElement ) ;
range.Select() ;
FCKSelection.Delete() ;

To note that "range" is FCKDomRange, not a standard DOM Range object.

Attachments (2)

collapsed_select_test.html (2.2 KB) - added by Frederico Caldeira Knabben 10 years ago.
TC. Changed it to make it easier to see the whitespace issue.
firebug-ctrl-b.png (9.4 KB) - added by Hallvord R. M. Steen (Opera Software) 10 years ago.
screen shot of Firebug DOM HTML view after several ctrl-b presses

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

I've even simplified the Opera implementation for it with [854]:

var collapsedElement = this.BuildElement( doc ) ;
collapsedElement.appendChild( doc.createTextNode('') ) ;
range.InsertNode( collapsedElement ) ;
range.MoveToNodeContents( collapsedElement ) ;
range.Select() ;

But we still have the undesired whitespace behavior. Not only, if we move the caret to the right with the keyboard right after pressing CTRL+B, when returning to the left, the cursor will fell inside the empty <b></b>, which is also not desired. It must simply be ignored when moving the caret, as it does with IE, FF and Safari.

To note that, empty <b></b> is ignored when loading a document with it. It is not being ignored only at our style case, probably because it contains an empty text node inside of it.

comment:2 Changed 10 years ago by Frederico Caldeira Knabben

I've attached a simple TC for it. It may be handy to Hallvord.

It shows both bugs:

  • The whitespace issue;
  • The caret positioning issue. This one happens only when moving the caret to the left over the empty element. Moving to the right is ok.

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: collapsed_select_test.html added

TC. Changed it to make it easier to see the whitespace issue.

comment:3 Changed 10 years ago by Hallvord R. M. Steen (Opera Software)

I can no longer reproduce the "extra whitespace on multiple ctrl-b" issue with that test case but I can in the editor :(

I don't quite understand the code for inserting/removing styles. It is very complex. As far as I can see, on Ctrl-B FCKCoreStyleCommand.Execute runs but IsActive is only false on the first Ctrl-B. On all subsequent calls it is true, so new keypresses will try to remove the B tag again and again, and apparently fail to do so. RemoveFromRange is called, it creates a "bookmark" SPAN inside the B tag and moves it outside the B tag, then removes it again.

According to Firebug, the empty B tags are not removed in Firefox either, but the input cursor is placed outside the tag. Is this how it is meant to work?

Changed 10 years ago by Hallvord R. M. Steen (Opera Software)

Attachment: firebug-ctrl-b.png added

screen shot of Firebug DOM HTML view after several ctrl-b presses

comment:4 Changed 10 years ago by Hallvord R. M. Steen (Opera Software)

Owner: set to Frederico Caldeira Knabben

I have filed this as Opera bug 287177 but I think you may want to fix it by reviewing the logic for removing formatting because it doesn't look right to me.

comment:5 in reply to:  3 Changed 10 years ago by Frederico Caldeira Knabben

Replying to hallvord@opera.com:

I can no longer reproduce the "extra whitespace on multiple ctrl-b" issue with that test case but I can in the editor :(

The TC is not really related to multiple whitespaces and CTRL+B. Actually, you should not use CTRL+B on the TC, as it will be triggering Operas CTRL+B action. In the TC you can not only one whitespace added when loading the page.

I don't quite understand the code for inserting/removing styles. It is very complex.

Ohh yes it is. It gave me lots of sleepless nights :)

As far as I can see, on Ctrl-B FCKCoreStyleCommand.Execute runs but IsActive is only false on the first Ctrl-B. On all subsequent calls it is true, so new keypresses will try to remove the B tag again and again, and apparently fail to do so. RemoveFromRange is called, it creates a "bookmark" SPAN inside the B tag and moves it outside the B tag, then removes it again.

According to Firebug, the empty B tags are not removed in Firefox either, but the input cursor is placed outside the tag. Is this how it is meant to work?

You have spotted a bug introduced with [865], actually if you try it right before that changeset, you will see that the <b> will go away (sorry but Opera is still disable there).

I'll work to fix it now, so you can properly test it again soon.

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

I've opened #1298 for the empty style removal issue.

comment:7 Changed 10 years ago by Frederico Caldeira Knabben

Priority: NormalHigh

comment:8 Changed 10 years ago by Frederico Caldeira Knabben

Owner: Frederico Caldeira Knabben deleted

comment:9 Changed 10 years ago by Hallvord R. M. Steen (Opera Software)

Owner: set to Hallvord R. M. Steen (Opera Software)
Status: newassigned

On Opera's side this is bug 287177. Following up on it now..

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

sadly still with us in build 9755

comment:11 Changed 9 years ago by Hallvord R. M. Steen (Opera Software)

Resolution: worksforme
Status: assignedclosed

This annoyance finally seems resolved and the fix (hopefully the final fix..the last of several attempts actually) is in builds - tested with internal build 9825.

comment:12 Changed 9 years ago by Hallvord R. M. Steen (Opera Software)

I noticed another minor issue while testing this - filed as #1994

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