Implement resizable popups




18 years ago
9 years ago


(Reporter: bugs, Unassigned)


(Blocks: 2 bugs)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(3 attachments, 3 obsolete attachments)

For floating windows, palettes, resizable autocomplete dropdown, etc. 

Patch in a moment.
This is a patch from around December 2000, brought up to date with the current
build. I've ifdef'ed out some code in nsPopupBoxObject.cpp for metric gathering
because this functionality has since been added to nsBoxObject (where it should
be), however I've left the code here for now to remind myself to make the
nsBoxObject code stash info in attributes. 
hyatt, sr=?

Comment 5

18 years ago
I'm working on popups right now (bringing them up to spec), and my patch
conflicts heavily with this patch.  I think we're largely doing the same thing
(I'm also defining an nsIPopupBoxObject), but I worry that the re-architecture
will affect how this has to be implemented.  I'd rather finish that (which
should only take 1-2 more days), land, and then land this shortly after that.

Comment 6

18 years ago
Specifically I'm allowing <popup>, <tooltip> and <menupopup> to be definable
anywhere without needing a popupset.
Created attachment 46706 [details] [diff] [review]
progress... still not quite done. merged with dave's changes.
The one item awaiting resolution is the use of dimension/position attributes on
the popup node to specify positioning and size. 

Here's my proposal:

- if width, height, left or top are specified in attributes, these values are
used as the position and dimensions at which to launch and size the popup to.
This is to support persistence of these values which will be of use to UI
elements such as palette windows and floating toolbars. 

- if these attributes are not set (in all other cases, e.g. tooltips, menus,
context menus), the values supplied to ShowPopup etc are used. 

The patch supplied makes nsPopupBoxObject::ShowPopup obey |left| and |top|
during creation, however it does not yet make the popup size to the values of
|width| and |height|. Nor are these values dynamic - the popup does not move or
size when you set these attributes to new values. 

To be consistent with other XUL, this could be supported although it's not
likely to be the best way to resize a popup smoothly. Experience with this has
shown the need to support sizing in two dimensions at once, hence |sizeTo| and
|moveTo|. These methods also minimize FE notification during attribute set. As a
result of these methods, I don't know that we want to support dynamic attribute
I'm marking this as a blocker as it is blocking progress in creation of a
generic inline edit facility, which is required in the Bookmarks Upgrade
currently under way. Bug for this feature coming soon...
Severity: normal → blocker
Priority: -- → P2
Target Milestone: --- → mozilla0.9.5
After discussion with hyatt, a todo list before completion:

- provide pass-thrus for the new nsIPopupBoxObject methods on the popup binding
in XML, in preparation for a finalized nsIDOMXULPopupElement interface.
- implement nsIDocumentObserver on nsMenuPopupFrame to listen for left/top
attribute changes so as to shift the popup around when the attributes are changed. 
- make 'width' and 'height' fields on BoxObject return attribute value first, if
- make 'screenX' and 'screenY' fields on BoxObject return attribute value (left
and top) first, if present. (*)
- implement 'left' and 'top' mappings for nsIDOMXULElement

(*) a potential problem here. .left and .top traditionally refer to client
coordinates. Thus far I've been using them as screen coordinates which is
probably wrong. Better solutions are:

Screen Coords       Client Coords
.screenX            .left (or .x, to be consistent with the interface)
.screenY            .top  (or .y, similarly)

Comment 10

18 years ago
Ben, boxes already look for left and top and dirty themselves accordingly. 
width/height/left/top should all be taken care of automatically.   They dirty
for a reflow, which will then call RepositionPopup, and you presumably look for
left/top there.

So this should just work.

Comment 11

18 years ago
use left and top.  for a popup it makes sense that they mean screen.
But the getters for .x, .y, .screenX and .screenY are on boxobject, not on
popupboxobject. Do you propose I override these methods in the popupboxobject
case and use a different attribute? 
Don't mind me, I'm confusing myself. New patch shortly.
Created attachment 46720 [details] [diff] [review]
dom/content/layout changes.
OK Dave, ready for your take on this stuff. I'd like to get the back end changes
in, then work on the details of the actual XBL binding. 

Comment 16

18 years ago
I'll look at this later today.

Comment 17

18 years ago
sr=hyatt, but hold off until 0.9.5.
Attachment #45912 - Attachment is obsolete: true
Attachment #45913 - Attachment is obsolete: true
Attachment #46706 - Attachment is obsolete: true
Attachment #46720 - Flags: superreview+
summarizing suggestions:

- code in ::Init routine to position popup intially based on attributes
- add err2 in cases where ToInteger return code is checked for validity
- add persist, ref, datasources as well to XULElement
- refer to inconsistency in left/top for popups/other elts in idl.

new patch tomorrow. 

jag also requests if (NS_FAILED(err)) return err; in preference to if 
(NS_SUCCEEDED(err)) { stuff }

More specifically, I requested this be rewritten:

+  return NS_SUCCEEDED(err) ? popupSet->ShowPopup(srcContent, 
+                                                 popupContent, 
+                                                 aXPos, aYPos, 
+                                                 popupType, 
+                                                 anchorAlign,
+                                                 popupAlign) : err;


if (NS_FAILED(err))
  return err;

return popupSet->ShowPopup(srcContent, popupContent,
                           aXPos, aYPos, popupType, 
                           anchorAlign, popupAlign);
Created attachment 48824 [details]
test case for MoveTo function and left/top attributes
jag sez 'r=jag'
back end changes checked in. 

As I don't plan on using a popup for inline edit anymore, at this stage the
0.9.5 section of this bug is complete. Shifting the remainder of the work to
.9.7 for now. 
Target Milestone: mozilla0.9.5 → mozilla0.9.7
Target Milestone: mozilla0.9.7 → Future
Severity: blocker → normal
No longer a blocker. 

Comment 26

16 years ago
two years further, still a lot of code commented, so in there
but not used. Will this be ever used? 
If not, then please remove it from the tree(s)...
Mass reassign of my non-Firefox bugs to
Assignee: bugs → ben_seamonkey
Mass reassign of my non-Firefox bugs to
Let's not remove code that there may be a use for in XUL2.0. If we don't
identify any use for this in XUL2.0, then it can be removed. But for now, let it
stay, at least in toolkit/content. 
Assignee: ben_seamonkey → nobody
QA Contact: jrgmorrison → xptoolkit.menus
Blocks: 78508


11 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets

Comment 30

9 years ago
This was fixed by bug 511180.
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 511180
You need to log in before you can comment on or make changes to this bug.