Opened 12 years ago

Last modified 3 years ago

#4782 assigned defect

[patch][ccla]IFrame relative offset problem in dojo.coords()

Reported by: guest Owned by: dylan
Priority: high Milestone: 1.15
Component: HTML Version: 0.9
Keywords: Cc: amimalik@…
Blocked By: Blocking:

Description (last modified by Douglas Hays)

It seems that when I want to use/draw a widget inside an IFrame Dojo (version 0.9.0) does not take into account the relative position of the IFrame in the parent page. This causes popups like Tooltips to appear outside of the IFrame. This testcase should demonstrate the behaviour:

<html>
<head>
<script type='text/javascript'
djConfig=' isDebug: true, usePlainJson: true' src='dojo/dojo.js'></script>

<script type='text/javascript'>
dojo.addOnLoad(function init() {
  window.frames["frm"].document.write("<html><body><div id='tip0'>hover</div></body></html>");
  var id = window.frames["frm"].document.getElementById('tip0');
  var c = dojo.coords(id);
  console.log("x: " + c["x"] + "  y: " + c["y"]);
});
</script>
</head>
<body class="tundra">
  <iframe name="frm" id="frm" width="300" height="300" frameborder="0"
    scrolling="yes" style="position:absolute;left:200px;top:200px;"></iframe>
</body>
</html>

The (x,y) coordinates of the element should take into account the IFrame offset.

Thank you!

  • Amir

Attachments (3)

test4782.html (1.2 KB) - added by Douglas Hays 12 years ago.
test html file with complex margins/border/padding demonstrating patched dojo.coords function
4782.patch (2.6 KB) - added by Douglas Hays 12 years ago.
possible fix - please review for inclusion in 1.1 - added optional includeFrame parameter to dojo.coords (to prevent breaking existing users) to compute absolute coordinates relative to document root instead of just to the nearest iframe (refreshed)
calc_frame_pos.patch (1.4 KB) - added by Wouter Hager 6 years ago.
calc the difference if node in frame

Download all attachments as: .zip

Change History (40)

comment:1 Changed 12 years ago by bill

Component: CoreHTML
Owner: changed from anonymous to sjmiles

comment:2 Changed 12 years ago by guest

any progress on this?

comment:3 Changed 12 years ago by guest

I am just curious if there is an expected fix date or release version in which this will be included? Thanks and happy new year!

comment:4 Changed 12 years ago by Douglas Hays

Keywords: doughays bill added
Milestone: 1.1
Priority: normalhigh

This is needed for an IBM product that will be using Dojo 1.1.

IBM reference # 78238.

I especially wanted to make sure that this scenario is supported by Dojo.

comment:5 Changed 12 years ago by bill

Priority: highnormal

comment:6 Changed 12 years ago by bill

Description: modified (diff)

See #5803 which may be a pre-req to this ticket.

Changed 12 years ago by Douglas Hays

Attachment: test4782.html added

test html file with complex margins/border/padding demonstrating patched dojo.coords function

comment:7 Changed 12 years ago by Douglas Hays

Cc: Douglas Hays bill added
Keywords: doughays bill removed

After applying the patch, I succesfully tested the test4782.html file using WinXP on the following:
FF2.0.0.12, FF3.0beta4, IE6, IE7, Opera9.26, Safari3.0.4

comment:8 Changed 12 years ago by Adam Peller

Summary: IFrame relative offset problem in dojo.coords()[patch][ccla]IFrame relative offset problem in dojo.coords()

Changed 12 years ago by Douglas Hays

Attachment: 4782.patch added

possible fix - please review for inclusion in 1.1 - added optional includeFrame parameter to dojo.coords (to prevent breaking existing users) to compute absolute coordinates relative to document root instead of just to the nearest iframe (refreshed)

comment:9 Changed 12 years ago by bill

Milestone: 1.11.2

Move all milestone 1.1 tickets to 1.2, except for reopened tickets and tickets opened after 1.1RC1 was released.

comment:10 Changed 12 years ago by guest

Perhaps this patch fixes the relative offset problem, but tooltips still do not correctly appear inside the IFrame, as demonstrated here:

<html>

<head>

<style type="text/css">

  @import "dijit/themes/tundra/tundra.css";

  @import "dojo/resources/dojo.css"

</style>

<script type='text/javascript'

djConfig='isDebug: true, usePlainJson: true' src='dojo/dojo.js'></script>



<script type='text/javascript'>

dojo.require("dijit.Tooltip");



dojo.addOnLoad(function init() {

  window.frames["frm"].document.write("<html><body><div id='tip0'>hover</div></body></html>");

  var id = window.frames["frm"].document.getElementById('tip0');

  var c = dojo.coords(id);

  console.log("x: " + c["x"] + "  y: " + c["y"]);



dojo.doc = window.frames["frm"].document; // HACK

var tip0 = new dijit.Tooltip({label:"programmatically created tooltip", connectId:["tip0"]});

dojo.doc = document; // ENDHACK



});

</script>

</head>

<body class="tundra">

  <iframe name="frm" id="frm" width="300" height="300" frameborder="0"

    scrolling="yes" style="position:absolute;left:200px;top:200px;"></iframe>

</body>

</html>

It seems like all the widgets would need to be updated to use Doug's patch? Perhaps a new bug should be opened for this...

comment:11 Changed 12 years ago by Douglas Hays

Description: modified (diff)

I'm beginning to think that instead of going all the way back to the root window, the default behavior should go back until the "dojo" window is encountered by default (in case that dojo is loaded in an iframe). Instead of a optional third parameter being boolean, it could be a window reference to stop on if the default is not desired. I'll code this up to see if it behaves better and w/o changing existing uses of dojo.coords.

comment:12 Changed 12 years ago by liucougar

that is basically what I implemented in dojo.html.getAbsolutePositionExt for dojo 0.4

see http://trac.dojotoolkit.org/browser/trunk/src/html/util.js#L49

comment:13 Changed 11 years ago by bill

Milestone: 1.21.3

move 1.2 bugs to 1.3

comment:14 Changed 11 years ago by bill

Milestone: 1.3future

comment:15 Changed 8 years ago by bill

Owner: changed from sjmiles to Eugene Lazutkin
Status: newassigned

comment:16 Changed 7 years ago by dylan

Milestone: future1.9
Owner: changed from Eugene Lazutkin to Douglas Hays

Is this still relevant?

comment:17 Changed 7 years ago by bawongfai

Yes. It is still relevant.

I try to put the tooltip into dijit Editor. The tooltip goes out of the Editor. Apparently the positioning algorithm is incorrect in this case.

comment:18 Changed 7 years ago by Douglas Hays

Cc: Douglas Hays bill removed
Milestone: 1.91.10

comment:19 Changed 6 years ago by Douglas Hays

Owner: Douglas Hays deleted
Status: assignedopen

comment:20 Changed 6 years ago by cjolif

Milestone: 1.10future

Nobody is assigned to this, waiting for soemone to step in pushing this back to a future release.

comment:21 Changed 6 years ago by Wouter Hager

I'd love to see this feature implemented for dom-geometry.position. I have a custom editor that uses an iframe to separate styles and application logic.

Last edited 6 years ago by Wouter Hager (previous) (diff)

comment:22 Changed 6 years ago by Wouter Hager

Perhaps this could be filed under dojo/dom-geometry position.

comment:23 Changed 6 years ago by dylan

wshager, if you care to update the patch against master and do a pull request, we'll review for 1.11 (we're only a couple of weeks out from 1.10 and it's probably too late for that release)

comment:24 Changed 6 years ago by Wouter Hager

Why is it too late when there's stuff to fix? I guess I just don't understand release cycles...

comment:25 Changed 6 years ago by dylan

Milestone: future1.10

Mostly based on the number of open tickets that have patches that need to be reviewed prior to our planned release date. That and no activity on it for 6 years until now. :)

If you have a patch by the end of the weekend, I'll look at it in time for the 1.10 release.

Changed 6 years ago by Wouter Hager

Attachment: calc_frame_pos.patch added

calc the difference if node in frame

comment:26 Changed 6 years ago by Wouter Hager

See attachment.

comment:27 Changed 6 years ago by Wouter Hager

I realize this patch would not be backwards-compatible with any previous solutions of calculating the iframe offset, so there should probably be an extra argument for position.

comment:28 Changed 6 years ago by bill

Owner: set to dylan
Status: openassigned

comment:29 Changed 6 years ago by Wouter Hager

Sorry, I might have been too eager to supply a patch. The win.global check will fail when the context is set to the iframe. Also, I have some doubts now if the extra code is worth the small gain.

comment:30 Changed 6 years ago by dylan

Milestone: 1.101.11

Ok, I think we'll need to delay this one to 1.11. I will look at it after we get the higher priority 1.10 items finished, assuming you want to refine your patch @wshager.

comment:31 Changed 6 years ago by Wouter Hager

I think the solution is quite simple: if we added some positionRelativeTo helper function, one of the targets could be the iframe. Considering this, I have no clue what the guidelines for such helper functions in dojo are. Can I just suggest away or is there some guidelines document? I have the impression you don't want functions to expand ad infinitum...

comment:32 Changed 5 years ago by Wouter Hager

Any update on this? I'm in the same predicament again. Considering the patch: since win.global is deprecated, what's the step?

comment:33 Changed 4 years ago by dylan

wshager, care to create a pull request for this?

comment:34 Changed 4 years ago by dylan

I would suggest we put the proposed patch into a PR, and let people give feedback. It's basically being ignored here unfortunately.

comment:35 Changed 4 years ago by Wouter Hager

I wouldn't mind creating a patch, but I had some questions that haven't been answered yet. I don't think I have the kind of insight into Dojo that I can provide a patch that simply works for all cases. Anyway, the questions where:

  • what to use instead of win.global?
  • where to put a helper function to calculate a relative position (e.g. to the iframe)?
  • should there even be a helper function? is the extra code worth the small gain?

You could also settle for a solution where the developer takes relative position into account.

Also note that the patch in the ticket is NOT the definitive solution.

comment:36 Changed 4 years ago by dylan

Milestone: 1.111.12

Ok, I'm going to punt for 1.12, and then we can try to work on a patch together @wshager.

comment:37 Changed 3 years ago by dylan

Milestone: 1.131.15

Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.

Note: See TracTickets for help on using tickets.