Closed Bug 793157 Opened 7 years ago Closed 7 years ago

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


(Core :: XUL, defect)

Not set





(Reporter: enndeakin, Assigned: enndeakin)




(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
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.
Attached patch updated patchSplinter Review
Attachment #663375 - Attachment is obsolete: true
Attachment #663534 - Flags: review?(neil)
Why not just call openPopup again to anchor it to a new element?
Bug 792295 comment 4 suggests that openPopup gives up too early for already-open panels.
Blocks: 793540
No longer blocks: 792295
(In reply to 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.
See also bug 798226 for a similar problem when the sizeTo method is used.
No longer blocks: 797209
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.

>+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.)
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
hrm, looks like this was landed without the r+ marked, and without the nit addressed?
Oops, I thought this was reviewed ok. I can address the comment, or backout as desired.
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.
Attachment #663534 - Flags: review?(neil) → review+
Component: XP Toolkit/Widgets: Menus → XUL
You need to log in before you can comment on or make changes to this bug.