793157 - Add a method to move a anchored popup to an new relative position

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Neil Deakin]

Attachment: patch

The attached patch adds:

popup.moveToAnchor(anchorElement, position, x, y, attributesOverride);

which anchors an open popup if it isn't already and sets the position and offsets accordingly.

Comment 1

[Neil Deakin]

Attachment: updated patch

Comment 2

[[e-mail]]

Why not just call openPopup again to anchor it to a new element?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Bug 792295 comment 4 suggests that openPopup gives up too early for already-open panels.

Comment 4

[Neil Deakin]

(In reply to [e-mail] from comment #2)
> Why not just call openPopup again to anchor it to a new element?

Because openPopup is used to open a popup, not move one.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

See also bug 798226 for a similar problem when the sizeTo method is used.

Comment 6

[[e-mail]]

Comment on attachment 663534 [details] [diff] [review]
updated patch

Sorry for the delay. The code looks good apart from the below nit but I haven't tried it out yet.

> NS_IMETHODIMP
>+nsPopupBoxObject::MoveToAnchor(nsIDOMElement* aAnchorElement,
>+                               const nsAString& aPosition,
>+                               int32_t aXPos, int32_t aYPos,
>+                               bool aAttributesOverride)
>+{
>+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
Unused. (Compare MoveTo which doesn't even bother.)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d0fe5f0aa25

Comment 8

[:Gavin Sharp [email: [e-mail]]]

hrm, looks like this was landed without the r+ marked, and without the nit addressed?

Comment 9

[Neil Deakin]

Oops, I thought this was reviewed ok. I can address the comment, or backout as desired.

Comment 10

[[e-mail]]

Comment on attachment 663534 [details] [diff] [review]
updated patch

2nd nit (but 1st nit is more important):

>+function runTest(id)
>+{
>+  $("popup").openPopup($("button1"), "after_start", 0, 0);
>+}
You take advantage of optional arguments here...

>+  popup.moveToAnchor($("button1"), "after_start", 0, 8, false);
...but you don't bother doing it here, when you could.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3ee9ad46c81c