Opened 16 years ago
Closed 16 years ago
#3089 closed Bug (fixed)
[IE]plugin:showblock problems
| Reported by: | Garry Yao | Owned by: | Alfonso Martínez de Lizarrondo |
|---|---|---|---|
| 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 (4)
Change History (15)
comment:1 Changed 16 years ago by
| Keywords: | IE8 added; IE removed |
|---|
Changed 16 years ago by
| Attachment: | 3089.patch added |
|---|
comment:2 Changed 16 years ago by
| Keywords: | Confirmed Review? added |
|---|---|
| Owner: | set to Garry Yao |
| Status: | new → assigned |
comment:3 follow-up: 4 Changed 16 years ago by
| 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 Changed 16 years ago by
| 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 16 years ago by
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.
comment:6 Changed 16 years ago by
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 16 years ago by
| Keywords: | Review? added; HasPatch Pending removed |
|---|
I'll adding a TC for '_cke_expando', which could be handled by other ticket.
comment:8 Changed 16 years ago by
| 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 :-)
comment:9 Changed 16 years ago by
| Keywords: | Review? added; Review- removed |
|---|---|
| Owner: | changed from Garry Yao to Alfonso Martínez de Lizarrondo |
| Status: | assigned → new |
Proposed new patch to fix the current testcases.
Changed 16 years ago by
| Attachment: | 3089_4.patch added |
|---|
comment:10 Changed 16 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
R+ for 3089_3, appending TC with the updated patch.
IE8 specific problem, which breaks CKEDITOR.dom.element::removeClass