Opened 10 years ago

Closed 10 years ago

#9439 closed defect (fixed)

[regression]: Editor toolbar Bold button failure in IE6/7

Reported by: Douglas Hays Owned by: Douglas Hays
Priority: high Milestone: 1.3.2
Component: Editor Version: 1.3.0
Keywords: Cc: Adam Peller
Blocked By: Blocking:

Description

Create a simple Editor with 4 duplicated lines and load using IE6 or 7.

<div dojoType="dijit.Editor">
ab345<br>
ab345<br>
ab345<br>
ab345<br>
</div>

Focus the Editor and double click the 3rd line to select it.
With the mouse, click the (B)old button in the toolbar and the text is NOT bolded. Select again and this time type ctrl+B and then the text is bold.
Note the other 3 lines work fine.
Also note that if the 4 lines contain ab34 instead of ab345 it also works OK.

Change History (11)

comment:1 Changed 10 years ago by bill

That's a weird one. It might have something to do with EnterKeyHandlingPlugin, I noticed that the value of the editor (from attr('value')) is strange, interspersing <p> and the original <br>'s:

<p>ab345</p>ab345<br /><p>ab345</p>ab345<br />

comment:2 Changed 10 years ago by Douglas Hays

also fails with plugins="bold?" added that removes the EnterKeyHandling? plugin

comment:3 Changed 10 years ago by Douglas Hays

plugins="['bold']"

comment:4 Changed 10 years ago by Douglas Hays

Summary: Editor toolbar Bold button failure in IE6/7[regression]: Editor toolbar Bold button failure in IE6/7

The problem seems to be caused by the removal of the isCollapsed check before calling _moveToBookmark in [13977].
liucougar, can you elaborate?
If I add it back, then this problem is fixed but I don't know what else is now broken.

_restoreSelection: function(){
        if(this._savedSelection){
                //only restore the selection if the current range is collapsed
                //if not collapsed, then it means the editor does not lose
                //selection and there is no need to restore it
                //if(dojo.withGlobal(this.window,'isCollapsed',dijit)){
                        //console.log('_restoreSelection true')
                        this._moveToBookmark(this._savedSelection);
                //}
                delete this._savedSelection;
        }
},

comment:5 Changed 10 years ago by liucougar

that's one of the limitations (bugs) of IE's range implementation: you save a bookmark of a range, then restore the range with moveToBookmark, however, in some cases, the restored range is slightly different from the original range

in this case, the restored range is different from the original range, and IE has problem to bold it

if isCollapsed checking is there, moveToBookmark won't be called, so the original range is kept, which works fine in IE

isCollapsed checking was removed, because it is useless if the editor is not in its own iframe. however, since then, the editor is moved back to an iframe, the isCollapsed checking is needed.

comment:6 Changed 10 years ago by Douglas Hays

Cc: Adam Peller added

I'd like to get this approved for 1.3.2 since it's a regression, is a normal code-path scenario, and is a tiny change.

comment:7 Changed 10 years ago by Adam Peller

sounds like continued fallout from the refactor rollback? As long as cougar is cool with it, I agree it should go in 1.3.x as well.

comment:8 Changed 10 years ago by Adam Peller

Milestone: tbd1.3.2

comment:9 Changed 10 years ago by Douglas Hays

Owner: changed from liucougar to Douglas Hays

comment:10 Changed 10 years ago by Douglas Hays

(In [18500]) References #9439. Restore check for isCollapsed during _restoreSelection (trunk).

comment:11 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [18501]) Fixes #9439. Restore check for isCollapsed during _restoreSelection (1.3.2).

Note: See TracTickets for help on using tickets.