As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 683855 - Implement input.{width,height}
: Implement input.{width,height}
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla16
Assigned To: Trevor Saunders (:tbsaunde)
:
: Andrew Overholt [:overholt]
Mentors:
http://www.w3.org/Bugs/Public/show_bu...
: 772207 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-01 02:49 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2013-08-10 14:03 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (6.23 KB, patch)
2011-09-01 07:03 PDT, Mounir Lamouri (:mounir)
Ms2ger: feedback+
Details | Diff | Splinter Review
patch 2 (10.84 KB, patch)
2012-06-26 22:19 PDT, Trevor Saunders (:tbsaunde)
bzbarsky: review+
Details | Diff | Splinter Review

Description User image :Ms2ger (⌚ UTC+1/+2) 2011-09-01 02:49:51 PDT
As discussed.
Comment 1 User image Mounir Lamouri (:mounir) 2011-09-01 07:03:00 PDT
Created attachment 557486 [details] [diff] [review]
Patch v1
Comment 2 User image Mounir Lamouri (:mounir) 2011-09-01 07:05:46 PDT
Try server push:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=9e3d51151949
Comment 3 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-09-05 07:11:36 PDT
Since .width/.height are mainly for the same use case what img has them, they should work the
same way, IMO.

Could you file a spec bug? (Or explain why the properties in <input type="image"> and <img> should work differently)
Comment 4 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-09-05 14:44:10 PDT
http://www.w3.org/Bugs/Public/show_bug.cgi?id=14036
Comment 5 User image Mounir Lamouri (:mounir) 2011-09-06 08:57:12 PDT
You are right Olli. I've been naive enough to believe the specs would mess up something that simple but indeed, IE has those two attributes reflected as numbers and having them behaving like the image element seems to make sense.
Comment 6 User image Mounir Lamouri (:mounir) 2011-10-28 01:45:25 PDT
The specs have changed and it should now behave like img.
Comment 7 User image Trevor Saunders (:tbsaunde) 2012-06-26 22:19:09 PDT
Created attachment 636999 [details] [diff] [review]
patch 2
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2012-06-26 22:36:17 PDT
Comment on attachment 636999 [details] [diff] [review]
patch 2

>+nsGenericHTMLElement::GetWidthHeight(imgIRequest *aRequest)

Let's call this "GetWidthHeightForImage".

>+++ b/content/html/content/src/nsGenericHTMLElement.h
>+   * Get width and hight, using given request if attributes are unset.

s/hight/height/ and s/request/image request/


>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>+nsHTMLInputElement::SetHeight(PRUint32 aHeight)
>+{
>+  nsAutoString value;
>+  value.AppendInt(aHeight);
>+
>+  return nsGenericHTMLElement::SetAttr(kNameSpaceID_None, nsGkAtoms::height,
>+                                       value, true);

Document that this is not using SetIntAttr because we have an unsigned integer?

Though maybe we should just add an overload of SetIntAttr that takes PRUint32....

>+nsHTMLInputElement::GetWidth(PRUint32 *aWidth)
>+return NS_OK;

Fix the indent, please.

>+++ b/dom/interfaces/html/nsIDOMHTMLInputElement.idl

Please rev the IID and fix the indentaion here too.

r=me with those nits and a better commit message.  ;)
Comment 9 User image Mounir Lamouri (:mounir) 2012-06-26 23:34:57 PDT
Thank you for working on that Trevor! :)
Comment 10 User image :Ms2ger (⌚ UTC+1/+2) 2012-06-27 00:53:31 PDT
(In reply to Boris Zbarsky (:bz) from comment #8)
> >+++ b/content/html/content/src/nsHTMLInputElement.cpp
> >+nsHTMLInputElement::SetHeight(PRUint32 aHeight)
> >+{
> >+  nsAutoString value;
> >+  value.AppendInt(aHeight);
> >+
> >+  return nsGenericHTMLElement::SetAttr(kNameSpaceID_None, nsGkAtoms::height,
> >+                                       value, true);
> 
> Document that this is not using SetIntAttr because we have an unsigned
> integer?
> 
> Though maybe we should just add an overload of SetIntAttr that takes
> PRUint32....

Something like SetUnsignedIntAttr(), maybe?
Comment 11 User image Boris Zbarsky [:bz] (still a bit busy) 2012-06-27 08:05:43 PDT
Seems fine to me.
Comment 12 User image Trevor Saunders (:tbsaunde) 2012-06-27 18:46:20 PDT
landed https://hg.mozilla.org/integration/mozilla-inbound/rev/b038d3cfa0c3
Comment 13 User image Ed Morley [:emorley] 2012-06-28 01:07:45 PDT
https://hg.mozilla.org/mozilla-central/rev/b038d3cfa0c3
Comment 14 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2012-07-09 13:49:29 PDT
*** Bug 772207 has been marked as a duplicate of this bug. ***

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