Opened 4 years ago

Closed 4 years ago

#12185 closed Bug (fixed)

[IE9QM] Error thrown when moving mouse over focused editor's scrollbar

Reported by: Piotrek Koszuliński Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.4.4
Component: General Version:
Keywords: IE Cc:

Description

  1. Open replacebycode sample after removing doctype from it (so IE9 works in quirks mode)
  2. Focus editor.
  3. Move mouse over editor's scroll bar.
  4. Error is thrown - Object doesn't support property or method 'contains'.

Not reproducible on IE8QM.

Change History (8)

comment:1 Changed 4 years ago by Artur Delura

Status: newconfirmed

comment:2 Changed 4 years ago by Piotrek Koszuliński

Error is thrown from `dom.element.contains()` method. We had few issues with DOM API failing when using domEvent.getTarget() from mouse event fired on scroll bar because the returned node was, if I remember correctly, the document or document element (html) or empty object. Anyway - type check or other kind of verification before using contains() is a must.

comment:3 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.3CKEditor 4.4.4

The 4.4.3 milestone was shortened, so the remaining tickets must be postponed.

comment:4 Changed 4 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:5 Changed 4 years ago by Marek Lewandowski

Status: assignedreview

The reason for that issue was DOMDocument.elementFromPoint() which is returning empty object in this case, that caused CKEDITOR.dom.element to be created with an empty object.

I was considering adding a following code:

if ( element && CKEDITOR.tools.objectCompare( element.$, {} ) ) {
	return null;
}

Rather than change elementFromPoint function, but turns out that isLine() handles null value gracefully, and other conditions are protected against null value.

Pushed to t/12185 at dev.

comment:6 Changed 4 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  1. Don't do object comparison - this is totally unnecessary. Check whether returned object has the type property - we use this duck typing a lot.
  2. Why have you added widget plugin to tests?

comment:7 Changed 4 years ago by Marek Lewandowski

Status: review_failedreview

Added mentioned changes.

Pushed to t/12185 at dev.

comment:8 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

I needed to amend the last commit because:

  • Native node has nodeType property, not type. Magicline was totally broken after that.
  • The test wasn't verifying anything, so I removed it. If you wrote test before fixing bug you would know that.

Fixed on master with git:20906e2.

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