Opened 8 years ago

Closed 8 years ago

#3804 closed Bug (fixed)

[IE] context menu position wrong for long document

Reported by: Garry Yao Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: UI : Context Menu Version:
Keywords: 3.0RC IE Review+ Cc:

Description (last modified by Garry Yao)

Reproducing Procedures

  1. Open the attached sample page in IE;
  2. Scroll the viewport to document end;
  3. Right click to show up context menu;
    • Expected Result: Context menu show up at the position where mouse click happened.
    • Actual Result: Context menu is not align correctly in view port.

Attachments (5)

sample_for_reproduce.html (4.8 KB) - added by Garry Yao 8 years ago.
Sample page for reproducing
3804.patch (1.4 KB) - added by Garry Yao 8 years ago.
3804_2.patch (2.4 KB) - added by Martin Kou 8 years ago.
3804_3.patch (4.5 KB) - added by Martin Kou 8 years ago.
3804_4.patch (4.8 KB) - added by Martin Kou 8 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by Garry Yao

Description: modified (diff)

Changed 8 years ago by Garry Yao

Attachment: sample_for_reproduce.html added

Sample page for reproducing

comment:2 Changed 8 years ago by Garry Yao

Keywords: 3.0RC Review? added
Owner: set to Garry Yao
Status: newassigned

Not sure about the patch, but found L1106 of element.js is causing IE offset to be wrong:

editorFrame.getBoundingClientRect() // Always returning top as -1

Changed 8 years ago by Garry Yao

Attachment: 3804.patch added

comment:3 Changed 8 years ago by Martin Kou

Keywords: Review- added; Review? removed

Forcing the scroll check for IE should be wrong.

We're looking for the coordinates of the <html> element in our case, and therefore adding the scrollLeft and scrollTop to it is senseless - the html element is supposed to be the thing containing the scrollbar, not the other way round.

However, if you consider the common case where we're looking for the position of something inside that <html> element, then the scroll check would make sense.

Also, since different iframes can be rendered in standards/quirks mode separately, relying on CKEDITOR.env.quirks is also wrong.

Changed 8 years ago by Martin Kou

Attachment: 3804_2.patch added

comment:4 Changed 8 years ago by Martin Kou

Owner: changed from Garry Yao to Martin Kou
Status: assignednew

comment:5 Changed 8 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:6 Changed 8 years ago by Martin Kou

Keywords: Review? removed

:(

Now the context menu is broken in non-IE browsers. Retracting the review request.

Changed 8 years ago by Martin Kou

Attachment: 3804_3.patch added

comment:7 Changed 8 years ago by Martin Kou

Keywords: Review? added

Changed 8 years ago by Martin Kou

Attachment: 3804_4.patch added

comment:8 Changed 8 years ago by Martin Kou

Status: newassigned

Updated patch with comments explaining the purpose of the needAdjustScrollAndBorders flag.

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:10 Changed 8 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [3742].

Click here for more info about our SVN system.

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