Fixed JavaScript menu: arrowPosRef option is incorrectly handled

[Arty]

Affected version

2.0.10

This is bug with JavaScript menus code.

In structure.js in XF.MenuClick's init() function menu is created. By default $menuPosRef and $arrowPosRef are both assigned to this.$target.

However there are options to assign custom reference points: menuPosRef and arrowPosRef.

Sample code (code is similar to code in color_picker.js, so you can test it on color picker by adding same options and changing menu origin):

            this.menu = new XF.MenuClick(this.$target, {
                menuPosRef: this.options.box,
                arrowPosRef: this.options.box
            });

It would be logical that resulting menu would use $box as both menu and arrow reference points. But actually it only changes menu reference point.

Bug is caused by this code in js/xf/core/structure.js (line 335 in XenForo 2.0.10):

					if (!this.$menuPosRef.has(this.$arrowPosRef).length)
					{
						this.$arrowPosRef = this.$target;
					}

That assertion fails if menuPosRef and arrowPosRef are identical.

By default they are already identical, yet by time that code is reached, $menuPosRef has been customised. Wouldn't it make more sense to set $arrowPosRef to $menuPosRef instead of $target?

I think correct fix is to replace that code with:

					if (!this.$menuPosRef.has(this.$arrowPosRef).length)
					{
						this.$arrowPosRef = this.$menuPosRef;
					}

 

[XF Bug Bot]

Thank you for reporting this issue. The issue is now resolved and we are aiming to include that in a future XF release (2.0.13).

Change log:

Allow the menu arrowPosRef option to point to the same element as menuPosRef

Any changes made as a result of this issue being resolved may not be rolled out here until later.

 

[Mike]

Just to clarify, the fix here isn't what's suggested exactly, but we do now explicitly allow you to set the arrowPosRef to point to the same element. Our default page container template actually tried to do this and it was ignored, falling back to the target. However, this is generally what we wanted, so I removed the data-arrow-pos-ref from those cases. (It has to do with how we position the arrow for the navigation entries.)