Ticket #3804 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

[IE] context menu position wrong for long document

Reported by: garry.yao Owned by: martinkou
Priority: Normal Milestone: CKEditor 3.0
Component: UI : Context Menu Version:
Keywords: 3.0RC IE Review+ Cc:

Description (last modified by garry.yao) (diff)

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

sample_for_reproduce.html (4.8 KB) - added by garry.yao 5 years ago.
Sample page for reproducing
3804.patch (1.4 KB) - added by garry.yao 5 years ago.
3804_2.patch (2.4 KB) - added by martinkou 5 years ago.
3804_3.patch (4.5 KB) - added by martinkou 5 years ago.
3804_4.patch (4.8 KB) - added by martinkou 5 years ago.

Change History

comment:1 Changed 5 years ago by garry.yao

  • Description modified (diff)

Changed 5 years ago by garry.yao

Sample page for reproducing

comment:2 Changed 5 years ago by garry.yao

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

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 5 years ago by garry.yao

comment:3 Changed 5 years ago by martinkou

  • 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 5 years ago by martinkou

comment:4 Changed 5 years ago by martinkou

  • Owner changed from garry.yao to martinkou
  • Status changed from assigned to new

comment:5 Changed 5 years ago by martinkou

  • Keywords Review? added; Review- removed

comment:6 Changed 5 years ago by martinkou

  • Keywords Review? removed

:(

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

Changed 5 years ago by martinkou

comment:7 Changed 5 years ago by martinkou

  • Keywords Review? added

Changed 5 years ago by martinkou

comment:8 Changed 5 years ago by martinkou

  • Status changed from new to assigned

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

comment:9 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:10 Changed 5 years ago by martinkou

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

Fixed with [3742].

Click here for more info about our SVN system.

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