Closed Bug 581947 Opened 10 years ago Closed 10 years ago

Show validation message error in the tooltip of an invalid element

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b5

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(2 files)

This is needed for bug 561636: when an element is invalid (and not barred from constraint validation), its tooltip should show the validation message error (element.validationMessage).
Attached patch Patch v1Splinter Review
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #460258 - Flags: superreview?(jonas)
Attachment #460258 - Flags: review?(gavin.sharp)
Depends on: 345624, 561634
Comment on attachment 460258 [details] [diff] [review]
Patch v1

This should probably get ui-review.
Attachment #460258 - Flags: review?(gavin.sharp) → review+
Attachment #460258 - Flags: ui-review?(limi)
Then we should get a screenshot showing what it looks like!
Attached image Screenshot
Here is a screenshot :)
(In reply to comment #4)
> Created attachment 460564 [details]
> Screenshot
> 
> Here is a screenshot :)

The mouse pointer isn't visible so you have to believe the mouse was hovering the textfield ;)
Comment on attachment 460258 [details] [diff] [review]
Patch v1

>@@ -1098,16 +1099,25 @@ DefaultTooltipTextProvider::GetNodeText(
>   NS_ENSURE_ARG_POINTER(aNode);
>   NS_ENSURE_ARG_POINTER(aText);
>     
>   nsString outText;
> 
>   PRBool lookingForSVGTitle = PR_TRUE;
>   PRBool found = PR_FALSE;
>   nsCOMPtr<nsIDOMNode> current ( aNode );
>+
>+  // If the element implement the constraint validation API,
>+  // show the validation message, if any, instead of the title.
>+  nsCOMPtr<nsIConstraintValidation> cvElement (do_QueryInterface(current));

Personally I prefer

nsCOMPtr<nsIConstraintValidation> cvElement = do_QueryInterface(current);

since I find it more readable, and it means the same thing in C++. I.e. it directly runs the nsCOMPtr ctor with the return value from do_QI. (weird quirk inherited from C).
Attachment #460258 - Flags: superreview?(jonas) → superreview+
Comment on attachment 460258 [details] [diff] [review]
Patch v1

Looks good to me from the screenshot, is there a tryserver build where I can test the flow of this when it's actually used? Alternatively, find me in the office since you're here, and walk me through it? There might be some interaction things that I can't read out of a screenshot we might want adjust. But generally, it looks good. :)
Attachment #460258 - Flags: ui-review?(limi) → ui-review+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/6a95227f6b07
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b5
You need to log in before you can comment on or make changes to this bug.