334939 - Implement click-and-hold for the Help viewer as well (move the patch from bug 333831 to Toolkit)

Status: RESOLVED INCOMPLETE

(Toolkit :: UI Widgets, defect)

Description

[Simon Bünzli]

The reimplemented click-and-hold from attachment #218514 [details] [diff] [review] (bug 333831) applies to Firefox' Back and Forward buttons only. That patch requires however that the same functionality be reimplemented wherever needed.

I suggest encapsulating click-and-hold functionality into a binding inheriting from toolbarbutton.xml#menu-button so that other users (such as the Help viewer or extensions) can take advantage of it as well.

NB: This bug is not about removing the drop-down markers (which IMO should remain at least as a visual indicator that a drop-down menu is available).

Comment 1

[Simon Bünzli]

Attachment: move click-and-hold functionality to toolbarbutton.xml

This patch undoes most of the checked-in patch to bug 333831 and moves the code into its own binding (inheriting from toolbarbutton.xml#menu-button). And it also applies click-and-hold functionality to the Help viewer.

Comment 2

[Asaf Romano (gone)]

Comment on attachment 219290 [details] [diff] [review]
move click-and-hold functionality to toolbarbutton.xml

There is no point in checking the pref after the timer callback is fired, the right place to check for it is the binding ctor.

Also, this isn't theme-specific stuff, how about a clickandhold attribute with a corresponding style rule in xul.css?

Comment 3

[Simon Bünzli]

(In reply to comment #2)
> (From update of attachment 219290 [details] [diff] [review] [edit])
> There is no point in checking the pref after the timer callback is fired, the
> right place to check for it is the binding ctor.

The problem with the constructor is that this would overwrite the constructor of button.xml#menu-button-base. Although that one just calls this.init(), it still seems hackish to rely such an implementation detail (hopefully XBL2 will allow to call parent constructors). I'd rather check on mouse-down otherwise (which keeps the pref effective immediately and not just for new windows).

> Also, this isn't theme-specific stuff, how about a clickandhold attribute with
> a corresponding style rule in xul.css?
Will that still work for #back-button/#forward-button in browser.xul? These get a new binding from browser.xml (Pinstripe and thus OSX only). And introducing a new attribute for such minor functionality seemed to be slightly overkill anyway.

Comment 4

[Simon Bünzli]

Attachment: allow to enable click-and-hold through an attribute

patch updated to comment #2

Now click-and-hold can be enabled on all platforms for toolbar menu-buttons by setting the clickandhold attribute to true. That attribute is set in browser.js/help.js onLoad (OSX only), so that the pref is only queried once per window (and no constructor magic is needed).

Comment 5

[Asaf Romano (gone)]

The Pinstripe change makes the browser.js part a no-op, doesn't it? In other words, the binding is applied anyway... Thinking about this more, I'm not sure we can use a separate binding without breaking click-and-hold on most browser themes.

Also, the help viewer alone (which, by the way, mimics the browser look and feel) isn't a good reason to maintain another XP binding; also note we don't have platform-specific bindings excluding the internals of the <wizard> binding. I'm still looking for use-cases here, I couldn't find anything in non-browser applications (not even the back and forward buttons of the Finder).

Comment 6

[Simon Bünzli]

(In reply to comment #5)
> The Pinstripe change makes the browser.js part a no-op, doesn't it?

No. While the binding is applied anyway, its handlers are no-ops unless the clickandhold attribute is set in browser.js.

> Thinking about this more, I'm not sure
> we can use a separate binding without breaking click-and-hold on most browser
> themes.

Theme specific bindings would have to inherit from toolbarbutton.xml#menu-button-click_hold instead of toolbarbutton.xml#menu-button (which is why the binding must be shipped for all platforms). Draw-back is that for most themes the click-and-hold mousedown handler will be futilely invoked. Advantage is that Safari/Mac-look-alike themes could easily get "native" click-and-hold functionality on all platforms.

> I'm still looking for use-cases here, I couldn't find anything in
> non-browser applications (not even the back and forward buttons of the Finder).

I suppose click-and-hold isn't consistently implemented on all OS X apps. As I understood it, click-and-hold should apply to all buttons where there's a menu available with what you can get by clicking the button multiple times.

That's the case in the help viewer (independently of what it tries to look like). That would also be the case for Multiple-Undo/Redo buttons (for an editor) or page flipping buttons (for e.g. print preview).

Comment 7

[Asaf Romano (gone)]

> (In reply to comment #5)
> > The Pinstripe change makes the browser.js part a no-op, doesn't it?
> 
> No. While the binding is applied anyway, its handlers are no-ops unless the
> clickandhold attribute is set in browser.js.

Ugh, I see. This is pretty bad from the API point of view.

> > Thinking about this more, I'm not sure
> > we can use a separate binding without breaking click-and-hold on most browser
> > themes.
> 
> Theme specific bindings would have to inherit from
> toolbarbutton.xml#menu-button-click_hold instead of
> toolbarbutton.xml#menu-button (which is why the binding must be shipped for all
> platforms). Draw-back is that for most themes the click-and-hold mousedown
> handler will be futilely invoked. Advantage is that Safari/Mac-look-alike
> themes could easily get "native" click-and-hold functionality on all platforms.
> 

You're assuming all themes will be updated to inherit from the new binding - I don't see that happening since most theme authors do not test their themes on OS X.
 
> > I'm still looking for use-cases here, I couldn't find anything in
> > non-browser applications (not even the back and forward buttons of the Finder).
> 
> I suppose click-and-hold isn't consistently implemented on all OS X apps. As I
> understood it, click-and-hold should apply to all buttons where there's a menu
> available with what you can get by clicking the button multiple times.
> 
> That's the case in the help viewer (independently of what it tries to look
> like). That would also be the case for Multiple-Undo/Redo buttons (for an
> editor) or page flipping buttons (for e.g. print preview).

[Going back three pages by choosing the third item in the back button menu is by no means similar to undoing the last three commands.]

I see click and hold (for the B&F buttons) a result of the orginal mouse gesture in NS4. Other uses of click and hold in OS X are very different (in particular, drag-delayed-drop).

Comment 8

[Simon Bünzli]

Attachment: click-and-hold functionality in the #menu-button binding

(In reply to comment #7)
> > No. While the binding is applied anyway, its handlers are no-ops unless the
> > clickandhold attribute is set in browser.js.
> Ugh, I see. This is pretty bad from the API point of view.

The only alternative which would be half-way reusable (and reusability is the intention of this bug) would be to have an additional clickAndHold.js in Toolkit which would provide a clickAndHoldify method which applies the handlers optionally. Would you prefer such a solution?

> You're assuming all themes will be updated to inherit from the new binding - I
> don't see that happening since most theme authors do not test their themes on
> OS X.

Good point. If the code were to reside in a binding, it would have to be the #menu-button binding itself thus (see this patch).

> [Going back three pages by choosing the third item in the back button menu is
> by no means similar to undoing the last three commands.]

As stated in bug 333831 comment #8: click-and-hold
"achieves an action akin to repeated clicking". This would be the case for Undo/Redo and page flipping buttons (among others).

Comment 9

[Simon Bünzli]

One more thing: having this functionality outside of a binding requires reapplying the handlers after toolbar customization. The current implementation of bug 333831 breaks when removing and readding the Back/Forward buttons to a toolbar (the code from delayedStartup would have to go to BrowserToolboxCustomizeDone as well).

And: should click-and-hold really apply for all mouse buttons?

Comment 10

[Simon Bünzli]

Lacking a proper concept as to where to support click-and-hold. -> INCOMPLETE