Opened 13 years ago

Closed 12 years ago

#6666 closed Bug (fixed)

Do not use element.all property (CKEditor entirely broken in latest Opera)

Reported by: Tony Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.6.3
Component: General Version: 3.4.2
Keywords: Opera Cc: Hallvord R. M. Steen (Opera Software), garryyao

Description

The editor demo at http://ckeditor.com/demo will not load in Opera. I am using Opera version 11.00 build 1060 available from http://my.opera.com/desktopteam/blog/ and CKEditor 3.4.2 (revision 6041.)

The error in Opera is :

JavaScript - http://ckeditor.com/demo
Timeout thread: delay 10 ms
Uncaught exception: TypeError: Cannot convert 'i.all' to object
Error thrown at line 18, column 3309 in <anonymous function: unselectable>() in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    while(j=i.all[k++])
called from line 136, column 0 in <anonymous function: build>(n, o) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    B.getChild([1,0,0,0,0]).unselectable();
called from line 24, column 1776 in <anonymous function>() in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    z.build(x);
called from line 22, column 1760 in <anonymous function: load>(u, v) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    k.call(l,q);
called via Function.prototype.call() from line 21, column 2595 in <anonymous function: load>(z) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    m.call(n,s,t);
called via Function.prototype.call() from line 21, column 2528 in <anonymous function: load>(l, m, n, o, p) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    u(true);
called from line 22, column 1216 in <anonymous function: load>(j, k, l) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    a.scriptLoader.load(o,function(u,v){if(v.length)throw '[CKEDITOR.resourceManager.load] Resource name "'+p[v[0]].join(',')+'" was not found at "'+v[0]+'".';for(var w=0;w<u.length;w++){var x=p[u[w]];for(var y=0;y<x.length;y++){var z=x[y];q[z]=this.get(z);m[z]=1;}}k.call(l,q);},this);
called from line 24, column 1643 in <anonymous function>(x) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    a.themes.load(y,function(){var z=x.theme=a.themes.get(y);z.path=a.themes.getPath(y);z.build(x);if(x.config.autoUpdateElement)v(x);});
called from line 24, column 1245 in <anonymous function>() in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    u(x);
called from line 21, column 2595 in <anonymous function: load>(z) in http://ckeditor.com/apps/ckeditor/3.4.2/ckeditor.js?1289422309:
    m.call(n,s,t);

I have attached a screen shot of the problem.

Attachments (3)

snapshot.png (71.8 KB) - added by Tony 13 years ago.
Screen shot
6666.patch (646 bytes) - added by Ms2ger 13 years ago.
Use getElementsByTagName
6666_2.patch (1.5 KB) - added by Frederico Caldeira Knabben 13 years ago.

Download all attachments as: .zip

Change History (18)

Changed 13 years ago by Tony

Attachment: snapshot.png added

Screen shot

comment:1 Changed 13 years ago by Frederico Caldeira Knabben

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

I've tested Opera builds 1026, 1055 and 1060 with the online demo, nightly builds and local trunk copies. All is working well.

Maybe Hallvord can give is a final confirmation.

comment:2 Changed 13 years ago by Tony

Fred, are you using the windows version? I suspect maybe a bug in opera's js rendering now if you're not seeing it but if I can narrow it down to just the Linux version, then I've got extra info I can report to the Opera people.

I've tried several times now, clearing cache, closing and then opening the browser, nightly demos, yet still the same results. CKEditor is not showing for me. :(

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

This is because we've experimentally removed element.all support in browser.js: http://my.opera.com/sitepatching/blog/show.dml/21280702

If you set opera:config#Browser%20JavaScript to 0, CKEditor should load fine again.

Now, the browser.js patch is merely testing the water prior to doing a proper core fix which we intended to ship later. The intention was to quickly get a feel for how many compat problems we'd run into (and man, we were really successful at that :-)).

We wanted to remove element.all because supporting it causes problems on some sites. I would like to ask if you can adjust your browser sniffing / object detection here and not make Opera use the method that depends on element.all (though I also fully realise that this code in an app with the number of legacy installations CKEditor has might mean that we can not ship any core fix and are stuck supporting element.all).

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

BTW: just to make this clear, the browser.js file with this patch was only released for preview/alpha builds (10.70, 11), so regular release builds everywhere are not and will not be affected by this experiment.

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

To track the progress / planning of the core fix, look at CORE-13301 "Remove the HTMLElement.all property" (should be available to those CKEditor developers who have access to Opera's BTS)

comment:6 Changed 13 years ago by Frederico Caldeira Knabben

Keywords: Opera added
Status: newconfirmed

@Hallvord, can you give some hints on how to patch Opera to remove element.all, so we can try it here and eventually bring a fix to the editor, even if Opera will have that change only in the future?

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

Sure Fred, what we did in browser.js was as simple as this:

HTMLElement.prototype.__defineGetter__('all', function(){});

Place that either in a user.js or temporarily paste it in the editor's code.

comment:8 Changed 13 years ago by Garry Yao

Cc: garryyao added

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

Summary: CKEditor entirely broken in latest OperaDo not use element.all property (CKEditor entirely broken in latest Opera)

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

BTW, this is where it needs fixing: http://dev.ckeditor.com/browser/CKEditor/trunk/_source/core/dom/element.js#L1219 I suggest iterating over a collection created with element.getElementsByTagName('*') instead.

Changed 13 years ago by Ms2ger

Attachment: 6666.patch added

Use getElementsByTagName

comment:11 Changed 13 years ago by Ms2ger

Keywords: HasPatch added

Changed 13 years ago by Frederico Caldeira Knabben

Attachment: 6666_2.patch added

comment:12 Changed 13 years ago by Frederico Caldeira Knabben

Owner: set to Frederico Caldeira Knabben
Status: confirmedreview

I wonder why we have things like sibling.$.all || sibling.$.getElementsByTagName( '*' ) in our range code, but the web tells me that we should not have problems with IE6+.

Thanks for the patch, Ms2ger.

comment:13 Changed 13 years ago by Ms2ger

Fred, any news here?

comment:14 Changed 12 years ago by Garry Yao

Keywords: HasPatch removed
Status: reviewreview_passed

comment:15 Changed 12 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.3
Resolution: fixed
Status: review_passedclosed

Fixed with [7405].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy