Opened 17 years ago
Closed 17 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)
Change History (21)
comment:1 Changed 17 years ago by
Keywords: | Confirmed Firefox added; Floating panel mozilla absolute position shared toolbar removed |
---|---|
Milestone: | → FCKeditor 2.5 |
Owner: | set to Martin Kou |
Status: | new → assigned |
comment:2 Changed 17 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:3 Changed 17 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 17 years ago by
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 17 years ago by
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 17 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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 17 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 follow-up: 13 Changed 17 years ago by
#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:
- Clicking on the panel or toolbar can disable the toolbar buttons.
- The color floating panels are sometimes too small when they're opened.
comment:9 Changed 17 years ago by
Another problem observed:
- It is sometimes possible to have both the text color and background color highlighted by opening them one after the other.
comment:10 Changed 17 years ago by
Yet another problem:
- 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.
comment:11 Changed 17 years ago by
Keywords: | Review? added; Confirmed Firefox removed |
---|
I've proposed a patch for fixing all the above mentioned problems:
- Toolbar floating panels not being displayed correctly when FCKeditor instances are contained within overflow: hidden containers.
- Toolbar floating panels not being positioned correctly when FCKeditor instances are positioned to the right of the panel button.
- Clicking on a panel button linked to an active toolbar panel twice disables the whole toolbar set.
- Clicking on one panel button and then another would lead to having two or more panel buttons highlighted when there are multiple FCKeditor instances.
- The border between a toolbar panel and its associated panel button is visible when there are multiple FCKeditor instances.
comment:12 Changed 17 years ago by
Milestone: | FCKeditor 2.5 → FCKeditor 2.6 |
---|---|
Version: | FCKeditor 2.5 Beta → SVN |
comment:13 Changed 17 years ago by
comment:14 Changed 17 years ago by
Keywords: | Review? removed |
---|
Changed 17 years ago by
Attachment: | 1514_2.patch added |
---|
comment:15 Changed 17 years ago by
Keywords: | Review? added |
---|
comment:16 Changed 17 years ago by
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 17 years ago by
testcase that I used, it should be added with the patch.
comment:17 Changed 17 years ago by
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 17 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Fixed with [1434].
Click here for more info about our SVN system.
Fixed with [1089].
Click here for more info about our SVN system.