I've reviewed the Jetpack panels JEP
along with the documentation on
the XUL panel
and the StickyWin
pages, and I've written up the following list of concerns and proposed
changes. Take a look, and let me know your thoughts.
My first and most serious concern is a note in the XUL panel
that mouse interaction with pages loaded into content
iframes in XUL panels doesn't work correctly due to bug 130078.
My initial thinking regarding implementation of the proposal was to
jetpack panel content in a content iframe inside a specially-styled XUL
panel, and if
that's not possible, then I'm not sure how to implement the proposal.
Perhaps we could insert a content iframe into the browser window and
then position it using absolute positioning, although that wouldn't
allow us to specify its frame. Maybe nested content iframes would work
for that, though, with the outer iframe creating the frame and the
inner one loading the panel content.
The JEP proposes jetpack.panel.open()
as the method call for
creating a panel, but I think we're better off making it a jetpack.Panel
constructor function, which separates instantiation from display (so a
jetpack can create a long-lived panel that it regularly updates but
only shows on user request), reduces overlap in the API wrt ways to
display a panel (cf. the show
method), and is consistent with
The JEP, like the XUL panel implementation, uses a single property to
specify both the point in the panel and the point in the element to
which the panel is attached (i.e. the "anchor" element) that determine
the position of the panel.
In XUL, this use of a single property makes positioning panels hard,
since it's not obvious which part of the property value represents
The JEP makes this better by requiring authors to prepend "with anchor"
to anchor element attachment point, but this
solution suffers from verbosity and additional parsing complexity.
The StickyWin web page panel implementation uses two different
properties to specify these points, and I think that's the best
approach, since it's relatively terse, clearly distinguishes the
points, and is relatively simple to parse.
The StickyWin names for these properties are "position" and "edge", and
those don't seem right, since they bear no relation to the elements for
whom they specify points. I'm not quite sure what the best terminology
would be, but since anchor
is a nautical term, cleat
comes to mind, f.e. used in the property names panelCleat
violates the anchor metaphor, however, since
anchors don't have cleats. Docks do, but dock already has a meaning in
UI terminology, so it might be confusing to reuse it here.
Positioning I18N Issues
The positioning property specifies left
value names. Those were deprecated in XUL some years back in favor of start
because elements are typically best positioned on the
opposite side in RTL locales than they are in LTR ones. CSS3 adopts
these names too in its text
(although it retains left
I suggest we employ start
emphasizing in the docs that they represent left
in LTR locales and the opposite in RTL ones.
Control Over Focus
The JEP proposes two properties for controlling focus, takeFocus
. takeFocus enables a jetpack to control
whether or not a panel gets focus when it is shown, while restoreFocus
lets a jetpack control whether or not the previously focused element
gets refocused when a panel is hidden.
seems reasonable, as there are valid use cases for
both possible values. In particular, panels introduced as the result of
user action and expectation (like clicking on a statusbar icon) should
get focus, while those that occur as the result of some other event
(like a change to some data that is periodically refreshed) should not.
However, we should be careful to guide authors in their use, and it's
worth considering whether there's some way to automate this by
detecting whether or not a panel is being opened by the user (similar
to how Firefox's popup blocker feature detects whether or not a request
to open a window was made by the user).
(It's also worth considering whether panels that open as the result of
some other event are actually notifications that should be specified
via a notifications JEP, since the use case for notifications is
different and should be handled separately.)
restoreFocus seems less reasonable, because it can leave the UI in a
state in which no element has focus, confounding keyboard-centric
users, and because the decision seems more dependent on the element
that had focus (which the jetpack knows nothing about) than the jetpack.
I suggest we retain takeFocus
, perhaps renaming it to simply focus
(or maybe focusOnShow
to make it clear that it happens when
you show the panel) but remove restoreFocus and have Jetpack make the
decision about whether to restore focus and what element to restore it
to, with the initial version always restoring focus to the previously
Alternately, let's remove both properties and write a notifications JEP
to handle the use cases for notifying users about events and target
panels to just those use cases in which user action opens a panel.
contentDocument and contentWindow
I'm not sure we need both these properties, as each is easily
obtainable from the other (contentDocument.defaultView
I expect that contentDocument
will be the more popular
property and the one we should provide, although we can note in the
docs how to get its window.
Also, just to confirm, do we really want the XPCNativeWrapper-wrapped
version of the document (i.e. the contentDocument
the XUL iframe
element)? I assume so, for security, but don't
want to assume something that has security implications.
And either way, why not just call it document
? As we aren't
providing both wrapped and unwrapped variants of this object, we don't
have to distinguish between them, and the one behaves enough like the
other. We should, of course, document the differences in behavior, but
I don't think the name is the key to that, and especially not the name contentDocument
We should encourage best practices for authors who use panels, like
when to use the focus properties (if we retain them) and how to make
panels togglable (i.e. by not moving the UI used to open the panel when
it opens, so users can close it again by clicking in the same place).
Closing a Panel
We should carefully consider and document how and when a panel can
close because of user action. First thoughts:
For panels that open as a result of user action and are focused when
they open, they close when the user presses escape or clicks outside
For panels that do not open as a result of user action and are not
focused when they open, they close when the user focuses them and then
presses escape or clicks outside the panel or when the user presses
their "close" button, which only appears for these kinds of panels.
Alternately, we could limit this JEP to only panels that open as the
result of user action and get focus, then handle the other use case as
part of a notifications JEP.
The use cases for the style property aren't clear. Since panels can
load any HTML, they can apply arbitrary style to the content they load
inside the HTML itself. The panel frame itself is likely to be minimal,
so opportunities to style it will also be minimal. And perhaps it
shouldn't be styleable, so multiple panels have consistent appearance,
helping to reinforce with users its consistent behavior (wrt. focus,
closing, etc.) across jetpacks.
If we keep the style
property, these properties might best be
applied via it. Otherwise, we should keep these properties, but it
might be worth adding a way for the content to determine the size of
the panel, just as statusbar panels can determine their width.
I wonder if it's possible to simplify the API for specifying the
content you want to load into the panel by making a content
setter that is polymorphic: if you pass it a URL, it loads the URL; if
you pass it some HTML, it displays that; and if you pass it some text,
it parses it as HTML or sniffs it to determine its content type.
XUL panels on Mac have triangular "arrows" that point from the panel to
its anchor element. StickyWin panels can acquire these via the
StickyWin.PointyTip class. I suggest we make Jetpack panels have these
on all OSes.