Fix popup position if popup is triggered from within an offsetParent element (#6887)

* Fix popup location for tables

This commit introduces the `popupAbsCoords` option to the $button widget
and implements an absolut coordinate format.

Coordinates for popups are stored in the format `(x,y,w,h)`. These
coordinates are relative to the offset parent of the element that
defines the popup.

This commits adds a second format `@(x,y,w,h)`. Coordinates specified in
this format a relative to the pages root element.

The `popupAbsCoords` option of the $button widget enables the use of
this coordinates.

* Unify the declaration of the RegEx for parsing the popup-position

The regular expression was declared in three locations with the same
content. This commit supplies a new function `parseCoordinates` in
`popup.js`. This function returns the parsed coordinates and understands
the classic/absolute coordinates.

This function is used in `reveal.js` and `action-popup.js` to parse the
coordinates.

* Add documentation for coordinate systems

* Consolidate creating coordinate strings

The Popup object now contains a `buildCoordinates` method that can be
used to build coordinate strings. It takes an "enum" for the coordinate-
system to use. This makes everything easily extensible and prevents the
use of magic values.

* Add tests for `parseCoordinates` and `buildCoordinates`

* Add `tv-popup-abs-coords` to `collectDOMVariables`

This will make the absolute coordinates available for the
`DraggableWidget` and the `EventCatcherWidget`.

* Add documentation for the `tv-popup-abs-coords`

... to the `DraggableWidget` and the `EventCatcherWidget`.

core/modules/utils/dom/dom.js

@@ -294,8 +294,21 @@ exports.collectDOMVariables = function(selectedNode,domNode,event) {
 		});
 		
 		if(selectedNode.offsetLeft) {
-			// Add a variable with a popup coordinate string for the selected node
-			variables["tv-popup-coords"] = "(" + selectedNode.offsetLeft + "," + selectedNode.offsetTop +"," + selectedNode.offsetWidth + "," + selectedNode.offsetHeight + ")";
+			// Add variables with a (relative and absolute) popup coordinate string for the selected node
+			var nodeRect = {
+				left: selectedNode.offsetLeft,
+				top: selectedNode.offsetTop,
+				width: selectedNode.offsetWidth,
+				height: selectedNode.offsetHeight
+			};
+			variables["tv-popup-coords"] = $tw.popup.buildCoordinates($tw.popup.coordinatePrefix.csOffsetParent,nodeRect);
+
+			var absRect = $tw.utils.extend({}, nodeRect);
+			for (var currentNode = selectedNode.offsetParent; currentNode; currentNode = currentNode.offsetParent) {
+				absRect.left += currentNode.offsetLeft;
+				absRect.top += currentNode.offsetTop;
+			}
+			variables["tv-popup-abs-coords"] = $tw.popup.buildCoordinates($tw.popup.coordinatePrefix.csAbsolute,absRect);
 
 			// Add variables for offset of selected node
 			variables["tv-selectednode-posx"] = selectedNode.offsetLeft.toString();

core/modules/utils/dom/popup.js

@@ -22,6 +22,19 @@ var Popup = function(options) {
 	this.popups = []; // Array of {title:,wiki:,domNode:} objects
 };
 
+/*
+Global regular expression for parsing the location of a popup.
+This is also used by the Reveal widget.
+*/
+Popup.popupLocationRegExp = /^(@?)\((-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+)\)$/
+
+/*
+Objekt containing the available prefixes for coordinates build with the `buildCoordinates` function:
+ - csOffsetParent: Uses a coordinate system based on the offset parent (no prefix).
+ - csAbsolute: Use an absolute coordinate system (prefix "@").
+*/
+Popup.prototype.coordinatePrefix = { csOffsetParent: "", csAbsolute: "@" }
+
 /*
 Trigger a popup open or closed. Parameters are in a hashmap:
 	title: title of the tiddler where the popup details are stored
@@ -136,8 +149,17 @@ Popup.prototype.show = function(options) {
 			height: options.domNode.offsetHeight
 		};
 	}
-	var popupRect = "(" + rect.left + "," + rect.top + "," + 
-				rect.width + "," + rect.height + ")";
+	if(options.absolute && options.domNode) {
+		// Walk the offsetParent chain and add the position of the offsetParents to make
+		// the position absolute to the root node of the page.
+		var currentNode = options.domNode.offsetParent;
+		while(currentNode) {
+			rect.left += currentNode.offsetLeft;
+			rect.top += currentNode.offsetTop;
+			currentNode = currentNode.offsetParent;
+		}
+	}
+	var popupRect = $tw.popup.buildCoordinates(options.absolute?$tw.popup.coordinatePrefix.csAbsolute:$tw.popup.coordinatePrefix.csOffsetParent,rect);
 	if(options.noStateReference) {
 		options.wiki.setText(options.title,"text",undefined,popupRect);
 	} else {
@@ -175,10 +197,49 @@ Popup.prototype.cancel = function(level) {
 Returns true if the specified title and text identifies an active popup
 */
 Popup.prototype.readPopupState = function(text) {
-	var popupLocationRegExp = /^\((-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+)\)$/;
-	return popupLocationRegExp.test(text);
+	return Popup.popupLocationRegExp.test(text);
 };
 
+/*
+Parses a coordinate string in the format `(x,y,w,h)` or `@(x,y,z,h)` and returns
+an object containing the position, width and height. The absolute-Mark is boolean
+value that indicates the coordinate system of the coordinates. If they start with
+an `@`, `absolute` is set and the coordinates are relative to the root element. If
+the initial `@` is missing, they are relative to the offset parent element and
+`absoute` is false.
+*/
+Popup.prototype.parseCoordinates = function(coordinates) {
+	var match = Popup.popupLocationRegExp.exec(coordinates);
+	if(match) {
+		return {
+			absolute: (match[1] === "@"),
+			left: parseFloat(match[2]),
+			top: parseFloat(match[3]),
+			width: parseFloat(match[4]),
+			height: parseFloat(match[5])
+		};
+	} else {
+		return false;
+	}
+}
+
+/*
+Builds a coordinate string from a coordinate system identifier and an object
+containing the left, top, width and height values.
+Use constants defined in the coordinatePrefix property to specify a coordinate
+system.
+If one of the parameters is invalid for building a coordinate string `(0,0,0,0)`
+will be returned.
+*/
+Popup.prototype.buildCoordinates = function(prefix,position) {
+	var coord = prefix + "(" + position.left + "," + position.top + "," + position.width + "," + position.height + ")";
+	if (Popup.popupLocationRegExp.test(coord)) {
+		return coord;
+	} else {
+		return "(0,0,0,0)";
+	}
+}
+
 exports.Popup = Popup;
 
 })();

core/modules/widgets/action-popup.js

@@ -57,20 +57,20 @@ Invoke the action associated with this widget
 */
 ActionPopupWidget.prototype.invokeAction = function(triggeringWidget,event) {
 	// Trigger the popup
-	var popupLocationRegExp = /^\((-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+)\)$/,
-		match = popupLocationRegExp.exec(this.actionCoords || "");
-	if(match) {
+	var coordinates = $tw.popup.parseCoordinates(this.actionCoords || "");
+	if(coordinates) {
 		$tw.popup.triggerPopup({
 			domNode: null,
 			domNodeRect: {
-				left: parseFloat(match[1]),
-				top: parseFloat(match[2]),
-				width: parseFloat(match[3]),
-				height: parseFloat(match[4])
+				left: coordinates.left,
+				top: coordinates.top,
+				width: coordinates.width,
+				height: coordinates.height
 			},
 			title: this.actionState,
 			wiki: this.wiki,
-			floating: this.floating
+			floating: this.floating,
+			absolute: coordinates.absolute
 		});
 	} else {
 		$tw.popup.cancel(0);

core/modules/widgets/button.js

@@ -173,6 +173,7 @@ ButtonWidget.prototype.triggerPopup = function(event) {
 	if(this.popupTitle) {
 		$tw.popup.triggerPopup({
 			domNode: this.domNodes[0],
+			absolute: (this.popupAbsCoords === "yes"),
 			title: this.popupTitle,
 			wiki: this.wiki,
 			noStateReference: true
@@ -180,6 +181,7 @@ ButtonWidget.prototype.triggerPopup = function(event) {
 	} else {
 		$tw.popup.triggerPopup({
 			domNode: this.domNodes[0],
+			absolute: (this.popupAbsCoords === "yes"),
 			title: this.popup,
 			wiki: this.wiki
 		});
@@ -223,6 +225,7 @@ ButtonWidget.prototype.execute = function() {
 	this.setField = this.getAttribute("setField");
 	this.setIndex = this.getAttribute("setIndex");
 	this.popupTitle = this.getAttribute("popupTitle");
+	this.popupAbsCoords = this.getAttribute("popupAbsCoords", "no");
 	this.tabIndex = this.getAttribute("tabindex");
 	this.isDisabled = this.getAttribute("disabled","no");
 	// Make child widgets
@@ -252,7 +255,7 @@ Selectively refreshes the widget if needed. Returns true if the widget or any of
 */
 ButtonWidget.prototype.refresh = function(changedTiddlers) {
 	var changedAttributes = this.computeAttributes();
-	if(changedAttributes.actions || changedAttributes.to || changedAttributes.message || changedAttributes.param || changedAttributes.set || changedAttributes.setTo || changedAttributes.popup || changedAttributes.hover || changedAttributes.selectedClass || changedAttributes.style || changedAttributes.dragFilter || changedAttributes.dragTiddler || (this.set && changedTiddlers[this.set]) || (this.popup && changedTiddlers[this.popup]) || (this.popupTitle && changedTiddlers[this.popupTitle]) || changedAttributes.setTitle || changedAttributes.setField || changedAttributes.setIndex || changedAttributes.popupTitle || changedAttributes.disabled || changedAttributes["default"]) {
+	if(changedAttributes.actions || changedAttributes.to || changedAttributes.message || changedAttributes.param || changedAttributes.set || changedAttributes.setTo || changedAttributes.popup || changedAttributes.hover || changedAttributes.selectedClass || changedAttributes.style || changedAttributes.dragFilter || changedAttributes.dragTiddler || (this.set && changedTiddlers[this.set]) || (this.popup && changedTiddlers[this.popup]) || (this.popupTitle && changedTiddlers[this.popupTitle]) || changedAttributes.popupAbsCoords || changedAttributes.setTitle || changedAttributes.setField || changedAttributes.setIndex || changedAttributes.popupTitle || changedAttributes.disabled || changedAttributes["default"]) {
 		this.refreshSelf();
 		return true;
 	} else if(changedAttributes["class"]) {

core/modules/widgets/reveal.js

@@ -94,6 +94,13 @@ RevealWidget.prototype.positionPopup = function(domNode) {
 		left = Math.max(0,left);
 		top = Math.max(0,top);
 	}
+	if (this.popup.absolute) {
+		// Traverse the offsetParent chain and correct the offset to make it relative to the parent node.
+		for (var offsetParentDomNode = domNode.offsetParent; offsetParentDomNode; offsetParentDomNode = offsetParentDomNode.offsetParent) {
+			left -= offsetParentDomNode.offsetLeft;
+			top -= offsetParentDomNode.offsetTop;
+		}
+	}
 	domNode.style.left = left + "px";
 	domNode.style.top = top + "px";
 };
@@ -183,19 +190,11 @@ RevealWidget.prototype.compareStateText = function(state) {
 };
 
 RevealWidget.prototype.readPopupState = function(state) {
-	var popupLocationRegExp = /^\((-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+)\)$/,
-		match = popupLocationRegExp.exec(state);
+	this.popup = $tw.popup.parseCoordinates(state);
 	// Check if the state matches the location regexp
-	if(match) {
+	if(this.popup) {
 		// If so, we're open
 		this.isOpen = true;
-		// Get the location
-		this.popup = {
-			left: parseFloat(match[1]),
-			top: parseFloat(match[2]),
-			width: parseFloat(match[3]),
-			height: parseFloat(match[4])
-		};
 	} else {
 		// If not, we're closed
 		this.isOpen = false;

editions/test/tiddlers/tests/test-popup.js

@@ -0,0 +1,71 @@
+/*\
+title: test-popup.js
+type: application/javascript
+tags: [[$:/tags/test-spec]]
+
+Tests some utility function of the Popup prototype.
+
+\*/
+(function(){
+
+/*jslint node: true, browser: true */
+/*global $tw: false */
+"use strict";
+
+describe("Popup tests", function() {
+
+	it("parseCoordinates should parse valid coordinates", function() {
+		var popup = new $tw.utils.Popup({
+			rootElement: $tw.fakeDocument.createElement("div")
+		});
+
+		expect(popup.parseCoordinates("(1,2,3,4)")).toEqual({absolute: false, left: 1, top: 2, width: 3, height: 4});
+		expect(popup.parseCoordinates("(1.5,2.6,3.7,4.8)")).toEqual({absolute: false, left: 1.5, top: 2.6, width: 3.7, height: 4.8});
+		expect(popup.parseCoordinates("@(1,2,3,4)")).toEqual({absolute: true, left: 1, top: 2, width: 3, height: 4});
+		expect(popup.parseCoordinates("@(1.5,2.6,3.7,4.8)")).toEqual({absolute: true, left: 1.5, top: 2.6, width: 3.7, height: 4.8});
+	});
+
+	it("parseCoordinates should not parse invalid coordinates", function() {
+		var popup = new $tw.utils.Popup({
+			rootElement: $tw.fakeDocument.createElement("div")
+		});
+
+		expect(popup.parseCoordinates("#(1,2,3,4)")).toEqual(false);
+		expect(popup.parseCoordinates("(1,2,3,4")).toEqual(false);
+		expect(popup.parseCoordinates("(1,2,3)")).toEqual(false);
+	});
+
+	it("buildCoordinates should create valid coordinates", function() {
+		var popup = new $tw.utils.Popup({
+			rootElement: $tw.fakeDocument.createElement("div")
+		});
+
+		var coordinates = {
+			left: 1.5,
+			top: 2.6,
+			width: 3.7,
+			height: 4.8
+		};
+
+		expect(popup.buildCoordinates(popup.coordinatePrefix.csOffsetParent, coordinates)).toEqual("(1.5,2.6,3.7,4.8)");
+		expect(popup.buildCoordinates(popup.coordinatePrefix.csAbsolute, coordinates)).toEqual("@(1.5,2.6,3.7,4.8)");
+	});
+
+	it("buildCoordinates should detect invalid input", function() {
+		var popup = new $tw.utils.Popup({
+			rootElement: $tw.fakeDocument.createElement("div")
+		});
+
+		var coordinates = {
+			left: "invalid",
+			top: 2.6,
+			width: 3.7,
+			height: 4.8
+		};
+
+		expect(popup.buildCoordinates(popup.coordinatePrefix.csOffsetParent, coordinates)).toEqual("(0,0,0,0)");
+		expect(popup.buildCoordinates("dummy", coordinates)).toEqual("(0,0,0,0)");
+	});
+});
+
+})();

editions/tw5.com/tiddlers/concepts/CoordinateSystems.tid

@@ -0,0 +1,42 @@
+created: 20220810201659784
+modified: 20220810201659784
+tags: Concepts
+title: Coordinate Systems
+type: text/vnd.tiddlywiki
+
+TiddlyWiki (primarily the RevealWidget) supports two coordinate systems for positioning popups (see PopupMechanism to learn more about popups).
+
+<<.from-version "5.2.4">> We introduced absolute coordinates that may not work with all extensions and plugins. For maximum backwards compatibility, use absolute coordinates only where necessary.
+
+!! Relative coordinate system
+
+The default coordinate system is relative to the nearest positioned ancestor element. This is either:
+
+* an element with a non-static position, or
+* a ''td'', ''th'', ''table'' in case the element itself is static positioned.
+
+For tiddlers the nearest positioned ancestor element mostly is the body of the tiddler. Read the next chapter to learn about the exceptions.
+
+Relative coordinates are expressed in the form ''(x,y,w,h)''. Where ''x'' and ''y'' represent the position and ''w'' and ''h'' the width and height of the element.
+
+!! Absolute coordinate system
+
+The relative coordinate system works flawless most of the time. Problems occure if the target element (for example, a popup) and the source element (the triggering button) do not share the same positioned ancherstor element. This is often the case if the popup is declared outside a table and the triggering button is declared within a table cell. In this case the coordiante systems have different origins and the popup will be displayed in the wrong location.
+
+Absolute coordinates can fix this problem by using the root element of the page (the upper-left corner of the page) as the origin of the coordinate system. Absolute coordinates are expressed in the form ''@(x,y,w,h)''. Where ''x'' and ''y'' represent the position and ''w'' and ''h'' the width and height of the element. The leading ''@''-symbol marks these coordinates as absolute.
+
+The ButtonWidget has an option (''popupAbsCoords'') to put absolute coordinates into the state tiddler. The DraggableWidget and the EventCatcherWidget provide the absolute coordinate of an event within the attribute `tv-popup-abs-coords`.
+
+
+!! Example
+
+The following example shows a popup that is triggerd from within a table cell. The table cell is the nearest positioned ancestor element. The popup was defined outside the table cell. The button using relative coordinates will open the popup in the wrong location because the button and the popup do not agree on the same coordinate system. Using absolute coordinates fixes this problem.
+
+<<wikitext-example-without-html '<$reveal type="popup" state="$:/state/CoordinateSampleReveal">
+<div class="tc-drop-down">
+Popup
+</div>
+</$reveal>
+
+| Table Row 1 |<$button popup="$:/state/CoordinateSampleReveal">Relative coordinates</$button>|
+| Table Row 2 |<$button popup="$:/state/CoordinateSampleReveal" popupAbsCoords="yes">Absolute coordinates</$button>|'>>

editions/tw5.com/tiddlers/mechanisms/PopupMechanism.tid

@@ -8,6 +8,7 @@ The popup mechanism allows blocks of content to be selectively displayed and pos
 
 * [[StateTiddlers|StateMechanism]] to record whether a popup is currently displayed or not
 * The RevealWidget to selectively display the popup content
+** <<.from-version "5.2.4">> For positioning the popups relative or absolute coordinates can be used. See [[Coordinate Systems]] for more information about usage and format.
 ** For "sticky" popups — those that don't close when clicking inside one — set the ''class'' attribute to `tc-popup-keep`
 * The ButtonWidget to trigger the display of the popup by setting the state tiddler appropriately
 

editions/tw5.com/tiddlers/widgets/ActionPopupWidget.tid

@@ -1,6 +1,6 @@
 caption: action-popup
 created: 20200303114556528
-modified: 20210501203451387
+modified: 20220815205132124
 tags: Widgets ActionWidgets
 title: ActionPopupWidget
 type: text/vnd.tiddlywiki
@@ -15,10 +15,11 @@ The ''action-popup'' widget is invisible. Any content within it is ignored.
 
 |!Attribute |!Description |
 |$state |The title of the state tiddler for the popup |
-|$coords |Optional coordinates for the handle to which popup is positioned (in the format `(x,y,w,h)`) |
+|$coords |Optional coordinates for the handle to which popup is positioned (see [[Coordinate Systems]] for the supported formats) |
 |$floating |<<.from-version "5.2.0">> Optional. Defaults to `no`. Set to `yes` to create a popup that must be closed explicitly. |
 
-<<.from-version "5.1.23">> If the ''$coords'' attribute is missing or empty then all popups are cancelled.
+<<.from-version "5.1.23">> If the ''$coords'' attribute is missing or empty then all popups are cancelled.<br/>
+<<.from-version "5.2.4">> The ''$coords'' attribute supports absolute and relative coordinates. See [[Coordinate Systems]] for more information.
 
 <<.tip "Delete the state tiddler for a floating popup to close it.">>
 

editions/tw5.com/tiddlers/widgets/ButtonWidget.tid

@@ -1,6 +1,6 @@
 caption: button
 created: 20131024141900000
-modified: 20211009121239795
+modified: 20220810192251345
 tags: Widgets TriggeringWidgets
 title: ButtonWidget
 type: text/vnd.tiddlywiki
@@ -37,6 +37,7 @@ The content of the `<$button>` widget is displayed within the button.
 |default |Default value if <<.attr set>> tiddler is missing for testing against <<.attr setTo>> to determine <<.attr selectedClass>> |
 |popup |Title of a state tiddler for a popup that is toggled when the button is clicked. See PopupMechanism for details |
 |popupTitle |Title of a state tiddler for a popup that is toggled when the button is clicked. In difference to the <<.attr popup>> attribute, ''no'' TextReference is used. See PopupMechanism for details |
+|popupAbsCoords |<<.from-version "5.2.4">> If set to ''yes'' writes absolute coordinates to the tiddler referenced by the <<.attr popup>>. If set to ''no'' (the default) uses relative coordinates. See [[Coordinate Systems]] for details |
 |aria-label |Optional [[Accessibility]] label |
 |tooltip |Optional tooltip |
 |class |An optional CSS class name to be assigned to the HTML element|

editions/tw5.com/tiddlers/widgets/DraggableWidget.tid

@@ -41,7 +41,8 @@ The LinkWidget incorporates the functionality of the DraggableWidget via the ''d
 |!Variables |!Description |
 |`modifier` |The [[modifier Variable]] contains the Modifier Key held while dragging |
 |`dom-*` |All DOM attributes of the node being dragged are made available as variables, with the prefix `dom-` |
-|`tv-popup-coords` |A co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node that's being dragged where the event originated |
+|`tv-popup-coords` |A relative co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node matching the selector where the event originated (see [[Coordinate Systems]] for more information) |
+|`tv-popup-abs-coords` |<<.from-version "5.2.4">> An absolute co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node matching the selector where the event originated (see [[Coordinate Systems]] for more information) |
 |`tv-selectednode-posx` |`x` offset position of the dragged DOM node |
 |`tv-selectednode-posy` |`y` offset position of the dragged DOM node  |
 |`tv-selectednode-width` |`offsetWidth` of the dragged DOM node |

editions/tw5.com/tiddlers/widgets/EventCatcherWidget.tid

@@ -1,5 +1,5 @@
 created: 20201123113532200
-modified: 20220507184043398
+modified: 20221012194222875
 tags: Widgets TriggeringWidgets
 title: EventCatcherWidget
 type: text/vnd.tiddlywiki
@@ -47,7 +47,8 @@ The following variables are made available to the actions:
 |`event-mousebutton` |The mouse button (if any) used to trigger the event (can be "left", "right" or "middle"). Note that not all event types support the mousebutton property |
 |`event-type` |The type property of the JavaScript event |
 |`event-detail-*` |Any properties in the detail attribute of the event are made available with the prefix `event-detail-` |
-|`tv-popup-coords` |A co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node matching the selector where the event originated |
+|`tv-popup-coords` |A relative co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node matching the selector where the event originated (see [[Coordinate Systems]] for more information) |
+|`tv-popup-abs-coords` |<<.from-version "5.2.4">> An absolute co-ordinate string that can be used with the ActionPopupWidget to trigger a popup at the DOM node matching the selector where the event originated (see [[Coordinate Systems]] for more information) |
 |`tv-widgetnode-width` |<<.from-version "5.2.3">> `offsetWidth` of the DOM node created by the eventcatcher widget |
 |`tv-widgetnode-height` |<<.from-version "5.2.3">> `offsetHeight` of the DOM node created by the eventcatcher widget |
 |`tv-selectednode-posx` |`x` offset position of the selected DOM node |

plugins/tiddlywiki/dynannotate/modules/dynannotate.js

@@ -191,7 +191,7 @@ DynannotateWidget.prototype.applyAnnotations = function() {
 				"tv-selection-posy": (bounds.top).toString(),
 				"tv-selection-width": (bounds.width).toString(),
 				"tv-selection-height": (bounds.height).toString(),
-				"tv-selection-coords": "(" + bounds.left + "," + bounds.top + "," + bounds.width + "," + bounds.height + ")"
+				"tv-selection-coords": $tw.popup.buildCoordinates($tw.popup.coordinatePrefix.csOffsetParent,bounds)
 			});
 			if(self.hasAttribute("popup")) {
 				$tw.popup.triggerPopup({