Last Comment Bug 769355 - Implement <input type=date> max attribute
: Implement <input type=date> max attribute
Status: RESOLVED FIXED
: doc-bug-filed
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla20
Assigned To: Raphael Catolino (:raphc)
:
Mentors:
Depends on: 769370 769385
Blocks: 769352 769357
  Show dependency treegraph
 
Reported: 2012-06-28 10:44 PDT by Raphael Catolino (:raphc)
Modified: 2013-04-27 13:27 PDT (History)
2 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (6.09 KB, patch)
2012-07-25 03:28 PDT, Raphael Catolino (:raphc)
mounir: feedback+
Details | Diff | Splinter Review
patch (6.18 KB, patch)
2012-08-08 08:23 PDT, Raphael Catolino (:raphc)
mounir: review+
Details | Diff | Splinter Review
patch (6.19 KB, patch)
2012-08-08 12:12 PDT, Raphael Catolino (:raphc)
raphael.catolino: review+
Details | Diff | Splinter Review
patch (6.17 KB, patch)
2012-08-08 12:13 PDT, Raphael Catolino (:raphc)
raphael.catolino: review+
Details | Diff | Splinter Review
patch (6.17 KB, patch)
2012-08-16 11:33 PDT, Raphael Catolino (:raphc)
no flags Details | Diff | Splinter Review

Description Raphael Catolino (:raphc) 2012-06-28 10:44:38 PDT

    
Comment 1 Raphael Catolino (:raphc) 2012-07-25 03:28:45 PDT
Created attachment 645693 [details] [diff] [review]
patch

This patch uses functions from the patch in bug 769370. As long as this one as not been r+ this one might change so a review might not be useful yet. Feedback would still be appreciated though.
Comment 2 Mounir Lamouri (:mounir) 2012-08-06 04:36:44 PDT
Comment on attachment 645693 [details] [diff] [review]
patch

Review of attachment 645693 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +1267,5 @@
>    nsAutoString maxStr;
>    GetAttr(kNameSpaceID_None, nsGkAtoms::max, maxStr);
>  
> +  double max;
> +  bool result = GetDoubleFromString(maxStr, max);

I know understand why you created that method in a previous patch. It will be indeed needed here. Though, I don't quite like the name.
What about: ConvertStringToNumber(), so it will match the specification's naming?
Comment 3 Mounir Lamouri (:mounir) 2012-08-06 04:39:40 PDT
Comment on attachment 645693 [details] [diff] [review]
patch

Review of attachment 645693 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +4577,5 @@
> +        double max = GetMaxAsDouble();
> +        MOZ_ASSERT(!MOZ_DOUBLE_IS_NaN(max));
> +
> +        maxStr.AppendFloat(max);
> +      } else {

else if (mType == NS_FORM_INPUT_DATE)
and:
} else {
MOZ_NOTREACHED();
}
Comment 4 Raphael Catolino (:raphc) 2012-08-08 08:23:08 PDT
Created attachment 650131 [details] [diff] [review]
patch
Comment 5 Mounir Lamouri (:mounir) 2012-08-08 09:35:06 PDT
Comment on attachment 650131 [details] [diff] [review]
patch

Review of attachment 650131 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +1275,5 @@
>    GetAttr(kNameSpaceID_None, nsGkAtoms::max, maxStr);
>  
> +  double max;
> +  bool result = ConvertStringToNumber(maxStr, max);
> +  return result ? max : MOZ_DOUBLE_NaN();

You can do:
return ConvertStringToNumber(maxStr, max) ? max : MOZ_DOUBLE_NaN();
Comment 6 Mounir Lamouri (:mounir) 2012-08-08 09:37:42 PDT
Comment on attachment 650131 [details] [diff] [review]
patch

Review of attachment 650131 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +4603,5 @@
> +        maxStr.AppendFloat(max);
> +      } else if (mType == NS_FORM_INPUT_DATE) {
> +        GetAttr(kNameSpaceID_None, nsGkAtoms::max, maxStr);
> +      } else {
> +        NS_NOTREACHED("Invalid input type");

nit: please change that to "Unexpected input type".
Comment 7 Raphael Catolino (:raphc) 2012-08-08 12:12:47 PDT
Created attachment 650247 [details] [diff] [review]
patch
Comment 8 Raphael Catolino (:raphc) 2012-08-08 12:13:42 PDT
Created attachment 650248 [details] [diff] [review]
patch
Comment 9 Raphael Catolino (:raphc) 2012-08-16 11:33:00 PDT
Created attachment 652508 [details] [diff] [review]
patch

Updated patch.
Comment 10 Mounir Lamouri (:mounir) 2012-12-28 04:58:09 PST
https://hg.mozilla.org/mozilla-central/rev/739b8e307069
Comment 11 Florian Scholz [:fscholz] (MDN) 2013-04-27 13:27:32 PDT
See bug 866440 for documentation.

Note You need to log in before you can comment on or make changes to this bug.