Make <hint> to work more like a tooltip

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 185417 [details] [diff] [review]
v.1

This is not yet perfect, but makes <hint> to work a bit better way.
Attachment #185417 - Flags: review?(allan)

Comment 2

14 years ago
(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

14 years ago
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.
Attachment #185417 - Flags: review?(allan) → review+
Created attachment 185483 [details] [diff] [review]
v.1 + comments
Attachment #185417 - Attachment is obsolete: true
Attachment #185483 - Flags: review?(aaronr)

Comment 5

14 years ago
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.
Attachment #185483 - Flags: review?(aaronr) → review+
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.