Opened 16 years ago

Closed 16 years ago

#1514 closed Bug (fixed)

Floating panels positioned incorrectly in Mozilla with shared toolbar

Reported by: Will Pierce Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.6
Component: UI : Floating Panel Version: SVN (FCKeditor) - Retired
Keywords: Review+ Cc:

Description

When using one or more editing areas with a detached/shared toolbar, the floating panels for color, font, etc. are positioned incorrectly for editor areas which are absolutely or relatively positioned on the page. I have confirmed this in Firefox v2.0.0.9 as well as the latest versions of Netscape and Safari for Windows. This is not an issue in IE 6 or 7. I have also confirmed that this is an issue with FCK versions 2.4 as well as 2.5 Beta.

It appears that these floating panels are positioned according to the X/Y coordinates of the editing area, rather than the coordinates of the appropriate toolbar button.

Below is a link to a simple test case, with one absolutely positioned editor instance and a detached toolbar. The editor version is 2.5b: http://www.trackto.com/dev/scratch/index.php

If necessary I can create other test instances and/or post source code. I would appreciate some advice or a resolution on this ASAP as it is a show-stopper for the project I am working on.

Thanks

Will Pierce

Attachments (3)

1514.patch (6.7 KB) - added by Martin Kou 16 years ago.
Proposed patch for fixing #1514.
1514_2.patch (6.8 KB) - added by Martin Kou 16 years ago.
Revised patch which fixes both #1514 and #538.
011.html (2.7 KB) - added by Alfonso Martínez de Lizarrondo 16 years ago.
testcase that I used, it should be added with the patch.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 16 years ago by Martin Kou

Keywords: Confirmed Firefox added; Floating panel mozilla absolute position shared toolbar removed
Milestone: FCKeditor 2.5
Owner: set to Martin Kou
Status: newassigned

comment:2 Changed 16 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [1089].

Click here for more info about our SVN system.

comment:3 Changed 16 years ago by Will Pierce

Resolution: fixed
Status: closedreopened

This almost works for me, but not quite. I've encountered 2 problems: The first is that the context menu is positioned incorrectly if you right click in the editing area. The second issue is that the positioning doesn't quite work if the target element for the FCK instance is inside a content area that has positioning. The entire contents of my example page are wrapped in a div called "mainContent", which is centered on the page using the style property "margin:0 auto;". The offsetLeft and offsetTop properties of this wrapper are factored into the position, making the panel appear in an incorrect position. You'll notice the position of the panel varies if you resize the window horizontally, as the margins are recalculated. Here is the example: http://www.trackto.com/dev/scratch/index.php?fcksource=true

As a "quick and dirty" workaround, I've modified the getDocumentPosition function in fcktools.js to only pull the offsetLeft and offsetTop of the target element, rather than calculating the offets of the element's parents. I replaced:

while ( curNode && curNode != w.document.body )
{				
   x += curNode.offsetLeft  - curNode.scrollLeft ;
   y += curNode.offsetTop - curNode.scrollTop ;
   curNode = curNode.offsetParent ;
}

with:

 x += curNode.offsetLeft  - curNode.scrollLeft ;
 y += curNode.offsetTop - curNode.scrollTop ;
 curNode = curNode.offsetParent ;

This works in the following example, which has the change compressed to the main JS file (fcksource is not used in the url): http://www.trackto.com/dev/scratch/index.php

I'm not sure if this will cause problems with other events that use this function, and this does not help the position of the context menu, so like I said it is more of a work-around than an actual fix.

I appreciate all of your hard work on this and I would also like to say that FCKEditor is an excellent product, so thanks.

Will Pierce

comment:4 Changed 16 years ago by Will Pierce

Just a quick update:

I fixed the context menu issue by adding an if statement to the fckpanel.js file. I replaced:

var nPos = FCKTools.GetDocumentPosition( FCKTools.GetElementWindow( positionedAncestor ), positionedAncestor ) ;
oPos.X -= nPos.x ;
oPos.Y -= nPos.y ;

with:

if(!this.IsContextMenu) 
{
  var nPos = FCKTools.GetDocumentPosition( FCKTools.GetElementWindow(   positionedAncestor ), positionedAncestor ) ;
  oPos.X -= nPos.x ;
  oPos.Y -= nPos.y ;
}

So the context menu will appear correctly in the second link above (the one without fcksource enabled in the url).

Thanks Will

comment:5 Changed 16 years ago by Will Pierce

One more update: The positions of the floating panels are also incorrectly calculated if the toolbar itself is explicitly positioned. I added another workaround that looks for and calculates the offsets of my toolbar ("yToolbar") and its wrapper ("toolbarWrapper"), and adjusts accordingly, and this is accurate to within a couple of pixels (but still not perfect). I'm not including my code modifications as it references the specific IDs of my toolbar and the wrapper, so it obviously would not provide any kind of universal fix for this issue. Here is the incorrectly positioned example: http://www.trackto.com/dev/scratch/index2.php?fcksource=true And the one with my workaround: http://www.trackto.com/dev/scratch/index2.php

So, if there is a way to dynamically factor in the toolbar position to the panel position calculation, that would be great.

Just out of curiosity, why is it that this works perfectly in IE but not in Mozilla? It seems like the floating panel positions should be calculated in reference to the button (color or font or whatever) itself rather than the editor area, if the toolbar is not attached. But, I'm sure it's more complicated than that. :)

Thanks

Will

comment:6 Changed 16 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

I've fixed both issues in [1091].

The menu positions in IE are correct because a separate set of positioning logic is used in IE. However that doesn't work in FF/Opera/Safari so I can't just fix the bug by using IE's positioning logic instead.

There is indeed a way to factor in both the toolbar position to the panel position calculation, and that's what [1091] does. The full details are too complicated to explain in a few words. But to put it simply, it amounts to getting both the toolbar's (x,y) coordinates and the menu's parent element's (x,y) coordinates relative to a common reference point, and doing simple arithmetics with those coordinates.

If you've found any further problems, feel free reopen this ticket and report it.

comment:7 Changed 16 years ago by Will Pierce

Resolution: fixed
Status: closedreopened

This is still not quite working correctly. In this example, the floating panels are incorrectly positioned if an editing instance is positioned to the right of the toolbar buttons: http://www.trackto.com/dev/scratch/index2.php

You will also notice another issue on this page, which is that the panels appear behind the toolbar and are therefore partially hidden. This is not a z-index issue, but rather it is due to the main wrapper for the editing area having an overflow value of 'auto' ('overflow:scroll would produce the same result). I want to have a page in which the user can scroll down to access additional editing instances, while keeping the toolbar visible at the top of the page. This is not possible under the current system, since the panels are appended to the editing instances' parent element, rather than the document body or the toolbar itself. I'm not sure if you can do anything about this or not, but it seems like what I want to do is pretty logical and it would be great if this functionality could be accommodated.

Thanks

Will

comment:8 Changed 16 years ago by Martin Kou

#1723 was discovered while I was working on this ticket.

Positioning issue aside, there are some more issue I've noticed with the text color and background color floating panels:

  1. Clicking on the panel or toolbar can disable the toolbar buttons.
  2. The color floating panels are sometimes too small when they're opened.

comment:9 Changed 16 years ago by Martin Kou

Another problem observed:

  1. It is sometimes possible to have both the text color and background color highlighted by opening them one after the other.

comment:10 Changed 16 years ago by Martin Kou

Yet another problem:

  1. The border between the floating panel and the text color button is visible when there are multiple instances of FCKeditor.

Upon today's investigation, I found the primary culprit to be a wrong assumption used in classes/fcktoolbarpanelbutton.js - it assumes there is only one panel linked to the toolbar button, when in fact each FCKeditor instance would create its own floating panel to be linked to the button. I've tried a few quickfixes to the code but none of the worked. It seems the toolbar panel button logic needs a moderately extensive rewrite to fix this.

Changed 16 years ago by Martin Kou

Attachment: 1514.patch added

Proposed patch for fixing #1514.

comment:11 Changed 16 years ago by Martin Kou

Keywords: Review? added; Confirmed Firefox removed

I've proposed a patch for fixing all the above mentioned problems:

  1. Toolbar floating panels not being displayed correctly when FCKeditor instances are contained within overflow: hidden containers.
  2. Toolbar floating panels not being positioned correctly when FCKeditor instances are positioned to the right of the panel button.
  3. Clicking on a panel button linked to an active toolbar panel twice disables the whole toolbar set.
  4. Clicking on one panel button and then another would lead to having two or more panel buttons highlighted when there are multiple FCKeditor instances.
  5. The border between a toolbar panel and its associated panel button is visible when there are multiple FCKeditor instances.

comment:12 Changed 16 years ago by Martin Kou

Milestone: FCKeditor 2.5FCKeditor 2.6
Version: FCKeditor 2.5 BetaSVN

comment:13 in reply to:  8 Changed 16 years ago by Martin Kou

Replying to martinkou:

#1723 was discovered while I was working on this ticket.

Stupid me... should be #1728 instead.

comment:14 Changed 16 years ago by Martin Kou

Keywords: Review? removed

Review request retracted because of an issue discovered in #538 in Linux/Firefox. Patch is being rewritten to solve both this ticket and #538.

Changed 16 years ago by Martin Kou

Attachment: 1514_2.patch added

Revised patch which fixes both #1514 and #538.

comment:15 Changed 16 years ago by Martin Kou

Keywords: Review? added

comment:16 Changed 16 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ added; Review? removed

I'm not sure if there's a little conflict in fckpanel.js with the changes done after this patch was generated, it did appear in red but it seems that it did merge properly after all. Just review the changes before committing them.

I'm gonna attach the test page so it is added to the /testcases folder with the rest of the patch, maybe you want to do any adjustment before committing it

Changed 16 years ago by Alfonso Martínez de Lizarrondo

Attachment: 011.html added

testcase that I used, it should be added with the patch.

comment:17 Changed 16 years ago by Martin Kou

There was actually a very similar test case written in the FCKtest directory in our SVN repository, here. Sorry I haven't mentioned it in the ticket.

I'm preparing to commit the patch now, but the handling of the test cases will need some more discussion.

comment:18 Changed 16 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

Fixed with [1434].

Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy