Ticket #3089 (closed Bug: fixed)

Opened 6 years ago

Last modified 6 years ago

[IE]plugin:showblock problems

Reported by: garry.yao Owned by: alfonsoml
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: SVN (FCKeditor) - Retired
Keywords: IE8 Review+ Cc:

Description

It's unable to switch off ShowBlock on IE.

Attachments

3089.patch (1.2 KB) - added by garry.yao 6 years ago.
3089_2.patch (1.4 KB) - added by alfonsoml 6 years ago.
Updated patch
3089_3.patch (1.2 KB) - added by alfonsoml 6 years ago.
Updated patch
3089_4.patch (2.8 KB) - added by garry.yao 6 years ago.

Change History

comment:1 Changed 6 years ago by garry.yao

  • Keywords IE8 added; IE removed

IE8 specific problem, which breaks CKEDITOR.dom.element::removeClass

Changed 6 years ago by garry.yao

comment:2 Changed 6 years ago by garry.yao

  • Status changed from new to assigned
  • Owner set to garry.yao
  • Keywords Confirmed Review? added

comment:3 follow-up: ↓ 4 Changed 6 years ago by alfonsoml

  • Keywords Review- added; Review? removed

As IE8 is still in beta testing I think that it's better to wait until the final release to verify that this bug is still present instead of adding a workaround that might cause other problems if the behavior of IE8 is changed.

Meanwhile, a ticket should be open at their Connect site with a simple testcase to verify the problem. Can you take care at least of creating the testcase, or would you rather work on other things?

comment:4 in reply to: ↑ 3 Changed 6 years ago by garry.yao

  • Keywords HasPatch Pending added; Confirmed Review- removed

Replying to alfonsoml:

As IE8 is still in beta testing I think that it's better to wait until the final release to verify that this bug is still present instead of adding a workaround that might cause other problems if the behavior of IE8 is changed.

Meanwhile, a ticket should be open at their Connect site with a simple testcase to verify the problem. Can you take care at least of creating the testcase, or would you rather work on other things?

Thanks, but IE8's already in RC, and it's supporting removeAttribute('class') sounds like a standard compliant feature rather than a bug, but it's nice idea to hold our supporting with IE8 until it got released.

comment:5 Changed 6 years ago by alfonsoml

The patch works OK in the final release of IE8, but I would change the detection from version == 8 to version >= 8 (ideally it should work based on feature/bug detection, but that is another task)

Anyway, the block in the hasAttributes function includes a case for '_cke_expando', and I don't know how to test if this is OK in IE8.

Changed 6 years ago by alfonsoml

Updated patch

comment:6 Changed 6 years ago by alfonsoml

I've provided an updated patch that also fixes #3182.

Changes with regards to the original:

  • As I've suggested, the detection is version >=8
  • Added protection in getAttribute to fix #3182
  • in hasAttributes the condition was missing the negation, so it had the reverse effect.

Anyway, I still don't know how to check properly that the rest of the staff like '_cke_expando' works properly.

comment:7 Changed 6 years ago by garry.yao

  • Keywords Review? added; HasPatch Pending removed

I'll adding a TC for '_cke_expando', which could be handled by other ticket.

comment:8 Changed 6 years ago by alfonsoml

  • Keywords Review- added; Review? removed

Sorry, when I used the automated tests they showed that the patch was wrong, the hasAttributes should be executed like any other IE (just due to that _cke_expando).

I'll try to use the tests from now on :-)

Changed 6 years ago by alfonsoml

Updated patch

comment:9 Changed 6 years ago by alfonsoml

  • Keywords Review? added; Review- removed
  • Status changed from assigned to new
  • Owner changed from garry.yao to alfonsoml

Proposed new patch to fix the current testcases.

Changed 6 years ago by garry.yao

comment:10 Changed 6 years ago by garry.yao

  • Keywords Review+ added; Review? removed

R+ for 3089_3, appending TC with the updated patch.

comment:11 Changed 6 years ago by alfonsoml

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

Fixed with [3393]

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