The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla19

Status

()

Core
XP Toolkit/Widgets: Menus
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Neil Deakin, Assigned: Neil Deakin)

Tracking

Trunk
mozilla19
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

10.52 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 663375 [details] [diff] [review]
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.
(Assignee)

Comment 1

5 years ago
Created attachment 663534 [details] [diff] [review]
updated patch
Attachment #663375 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #663534 - Flags: review?(neil)

Comment 2

5 years ago
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.

Updated

5 years ago
Blocks: 793540

Updated

5 years ago
No longer blocks: 792295
(Assignee)

Comment 4

5 years ago
(In reply to neil@parkwaycc.co.uk 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.
Blocks: 797209
No longer blocks: 797209

Comment 6

5 years ago
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.)
https://hg.mozilla.org/mozilla-central/rev/3d0fe5f0aa25
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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?
(Assignee)

Comment 9

5 years ago
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+
https://hg.mozilla.org/mozilla-central/rev/3ee9ad46c81c
You need to log in before you can comment on or make changes to this bug.