[Webkit-unassigned] [Bug 21554] Hovering the "src" attribute for an image should show the image dimensions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21554


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43866|                            |review-
               Flag|                            |




--- Comment #13 from Pavel Feldman <pfeldman at chromium.org>  2009-11-25 14:10:14 PST ---
(From update of attachment 43866)
A handful of style nits, otherwise good to go. Please put // TODO: Replace with
InjectedScriptAccess.getBasicProperties(obj, [names]) in place of the extended
Q&A block. That will save few bytes on loading / parsing.

>  
> +WebInspector.ElementsTreeElement.createTooltipForImageNode = function(properties)
> +{

It is Ok to define this one of prototype even though it does not use 'this'. We
do that in other places.

> +    var tooltipText = null;
> +    var offsetHeight = properties["offsetHeight"];

You can use properties.offsetHeight (and there is no need in explicit
intermediate variable I guess).


>  WebInspector.ElementsTreeElement.prototype = {
>      get highlighted()
>      {
> @@ -787,14 +801,30 @@ WebInspector.ElementsTreeElement.prototy
>          if (this._editing)
>              return;
>  

> +            return callback();

No need to return it, just call it.

> +        var objectProxy = new WebInspector.ObjectProxy(this.representedObject.id);
> +        WebInspector.ObjectProxy.getPropertiesAsync(objectProxy, callback);

It would be even better if you provided the names of the properties needed
here, so that it would
be easier to understand what the 'properties' object you carry is needed for.



> +        var result = [];
> +        for (e in propertiesPayload)

We typically iterate over arrays using index 'for (var i = 0; i <
properties.length; ++i)'

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.