296714 - Make <hint> to work more like a tooltip

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Patch coming.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v.1

This is not yet perfect, but makes <hint> to work a bit better way.

Comment 2

[Allan Beaufour]

(In reply to comment #1)
> Created an attachment (id=185417) [edit]
> v.1
> 
> This is not yet perfect, but makes <hint> to work a bit better way.

... an explanation of what you exactly do in the patch would be nice. No need to
add it now, I'll figure it out, just a note.

Comment 3

[Allan Beaufour]

Comment on attachment 185417 [details] [diff] [review]
v.1

Note to myself: Do not write review comments in a buggy browser (it just
crashed on me...) :-(

>Index: nsXFormsMessageElement.cpp
>===================================================================
>+    targ->RemoveEventListener(NS_LITERAL_STRING("xforms-moz-hint-off"), this, PR_FALSE);
wrap

>+    targ->AddEventListener(NS_LITERAL_STRING("xforms-moz-hint-off"), this, PR_FALSE);

wrap

> nsXFormsMessageElement::HandleEphemeralMessage(nsIDOMDocument* aDoc,
>                                                nsIDOMEvent* aEvent)

Please add a comment about what you do here:
>+  if (mType == eType_Hint) {
>+    nsCOMPtr<nsIDocument> doc(do_QueryInterface(aDoc));
>+    if (!doc)
>+      return NS_OK;
>+
>+    nsXFormsMessageElement *msg =
>+    NS_STATIC_CAST(nsXFormsMessageElement*,
>+                   doc->GetProperty(nsXFormsAtoms::messageProperty));

>+    if (msg && msg == this) {

No need to check for |msg|, |this| better not be nsnull.

Nice patch. It fixes some of my original complaints.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v.1 + comments

Comment 5

[aaronr]

Comment on attachment 185483 [details] [diff] [review]
v.1 + comments

My only additional comment would be that maybe you want to centralize all of
the ephemeral hiding into one function.  Looks like that is what HideEphemeral
is meant for.  If it isn't generic enough, maybe move the timer canceling,
property unsetting and style setting into a central function and have both
HideEphemeral and HandleEphemeralMessage call it.

But this patch looks good as it is, too.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

checked in