Ticket #3089 (closed Bug: fixed)
[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
Change History
comment:2 Changed 4 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 4 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 4 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 4 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.
comment:6 Changed 4 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 4 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 4 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 :-)
comment:9 Changed 4 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.
comment:10 Changed 4 years ago by garry.yao
- Keywords Review+ added; Review? removed
R+ for 3089_3, appending TC with the updated patch.
comment:11 Changed 4 years ago by alfonsoml
- Status changed from new to closed
- Resolution set to fixed
Fixed with [3393]
IE8 specific problem, which breaks CKEDITOR.dom.element::removeClass