Permalink
Browse files

A few initial fixes for displaing HTML elements in the Watch ppanel

  • Loading branch information...
1 parent d63523b commit 5c220f5ea1045e7921dd92d8c255e9e4b35b85d9 @janodvarko janodvarko committed Jan 31, 2013
View
10 extension/content/firebug/chrome/domTree.js
@@ -96,12 +96,12 @@ DomTree.prototype = domplate(
getValueTag: function(member)
{
// xxxHonza: |this| is wrong at this moment (callback from Domplate uses wrong context).
+ // That's why we access the provider through the 'member' object.
+ // xxxHonza: It should be possible to provide the tag through a decorator or provider.
- // Get proper UI template for the value.
- // xxxHonza: The value can be fetched asynchronously so, the value tag
- // should be also provided (by a decorator or provider).
- var value = this.getValue(member);
- var valueTag = Firebug.getRep(value);
+ // Get proper template for the value. |member.value| should refer to remote
+ // object implementation.
+ var valueTag = Firebug.getRep(member.value);
return valueTag.tag;
},
View
7 extension/content/firebug/chrome/reps.js
@@ -871,8 +871,6 @@ function instanceOf(object, Klass)
return false;
}
-
-
// ********************************************************************************************* //
FirebugReps.Element = domplate(Firebug.Rep,
@@ -1163,6 +1161,11 @@ FirebugReps.Element = domplate(Firebug.Rep,
supportsObject: function(object, type)
{
+ // Remote objects can't use instanceof operand so, they use 'type' instead.
+ // All HTML element types starts with 'HTML' prefix.
+ if (type.indexOf("HTML") == 0)
@simonlindholm
simonlindholm Jan 31, 2013

Str.hasPrefix. (Which in some future I guess we'll search-replace with ES6 .startsWith.)

I'm pretty surprised that this works. I'd have thought that you'd have to totally rewrite the rep system.

@janodvarko
janodvarko Feb 1, 2013

Str.hasPrefix.

done at 7696734

I'd have thought that you'd have to totally rewrite the rep system.

It's not done yet and some changes will be necessary, but I'd like to avoid total rewrite of the rep
system (we are already having a lot of work with JSD2)

Honza

@simonlindholm
simonlindholm Feb 5, 2013

Is it possible for a first non-remotable iteration to just grab the underlying object of the grip? Then we could get a first version working a lot faster, and given that the architecture is still in place there doesn't seem to be any downsides.

@janodvarko
janodvarko Feb 5, 2013

That's what I want to do, but I am blocked by:
https://bugzilla.mozilla.org/show_bug.cgi?id=837723

Honza

@simonlindholm
simonlindholm Feb 5, 2013

Ah, so you can get Debugger.Object objects, just not the real ones. I hit the same problem in the closure inspector; my solution was this hack: https://github.com/firebug/firebug/blob/7a76f33d11b49746c3ef859502763a25849c6b65/extension/content/firebug/console/closureInspector.js#L122

+ return true;
+
return object instanceof window.Element;
},
View
2 extension/content/firebug/debugger/grips/gripProvider.js
@@ -36,7 +36,7 @@ GripProvider.prototype =
if (Obj.isFunction(object.hasProperties))
return object.hasProperties();
- var children = this.getChildren();
+ var children = this.getChildren(object);
return children && children.length > 0;
},
View
10 extension/content/firebug/debugger/grips/objectGrip.js
@@ -4,8 +4,9 @@ define([
"firebug/lib/trace",
"firebug/debugger/rdp",
"firebug/lib/promise",
+ "firebug/lib/array",
],
-function (FBTrace, RDP, Promise) {
+function (FBTrace, RDP, Promise, Arr) {
// ********************************************************************************************* //
// Object Grip
@@ -26,6 +27,9 @@ ObjectGrip.prototype =
getType: function()
{
+ if (!this.grip)
+ return "";
+
if (this.grip.prototype)
return this.grip.prototype["class"];
@@ -50,7 +54,7 @@ ObjectGrip.prototype =
// request. Let's use these properties for the value label.
// See also {@ObjectGrip}
if (this.grip.ownProperties)
- return this.grip.ownProperties;
+ return Arr.values(this.grip.ownProperties);
return {type: this.grip.type};
},
@@ -143,6 +147,8 @@ function createGripProxy(grip)
{
// xxxHonza: this is the place where we can use proxies so, Grips are working
// in DOM panel automatically
+ // xxxHonza: in case the grip represents an array the proxy should also
+ // be an array.
var obj = {};
for (var i=0; i<grip.properties.length; i++)
{
View
54 extension/content/firebug/debugger/grips/remoteNodeListRep.js
@@ -0,0 +1,54 @@
+/* See license.txt for terms of usage */
+
+define([
+ "firebug/lib/trace",
+ "firebug/lib/array",
+ "firebug/debugger/rdp",
+ "firebug/lib/promise",
+ "firebug/chrome/reps",
+ "firebug/lib/domplate",
+ "firebug/debugger/grips/objectGrip",
+],
+function (FBTrace, Arr, RDP, Promise, FirebugReps, Domplate, ObjectGrip) {
+with (Domplate) {
+
+// ********************************************************************************************* //
+// RemoteNodeListRep
+
+var RemoteNodeListRep = domplate(FirebugReps.ArrayLikeObject,
+{
+ getTitle: function(obj, context)
+ {
+ return "NodeList";
+ },
+
+ longArrayIterator: function(list)
+ {
+ // xxxHonza: the conversion of the list (map) to an array is the reason
+ // why there is this extra template. It would be better to directly reuse
+ // the ArrayLikeObject and just modify the supportsObject.
+ var array = Arr.values(list);
+ return this.arrayIterator(array, 300);
+ },
+
+ shortArrayIterator: function(list)
+ {
+ var array = Arr.values(list);
+ return this.arrayIterator(array, Options.get("ObjectShortIteratorMax"));
+ },
+
+ supportsObject: function(object, type)
+ {
+ return (type == "RemoteNodeListRep")
+ }
+});
+
+// ********************************************************************************************* //
+// Registration
+
+Firebug.registerRep(RemoteNodeListRep)
+
+return RemoteNodeListRep;
+
+// ********************************************************************************************* //
+}});
View
1 extension/content/firebug/debugger/main.js
@@ -20,6 +20,7 @@ define([
"firebug/debugger/grips/functionGrip",
"firebug/debugger/commands",
"firebug/remoting/debuggerClientModule",
+ "firebug/debugger/grips/remoteNodeListRep",
],
function(FBTrace, ObjectGrip) {
View
10 extension/content/firebug/firebug.js
@@ -1267,12 +1267,16 @@ window.Firebug =
getRep: function(object, context)
{
var type = typeof(object);
- if (type == 'object' && object instanceof String)
- type = 'string';
+ if (type == "object" && object instanceof String)
+ type = "string";
- // Support for objects with dynamic type info.
+ // Support for objects with dynamic type info. Those objects are mostly remote
+ // objects coming from the back-end (server side). We can't use |instanceof|
+ // operand for those objects and so we need to provide type.
if (object && Obj.isFunction(object.getType))
type = object.getType();
+ else if (object && object["class"])
+ type = object["class"];
@simonlindholm
simonlindholm Jan 31, 2013

Doesn't this touch user-level objects...? object instanceof Object might work as a work-around.

Rather than abusing the "type" parameter for storing [[Class]] values of remote objects I think we should introducing a new parameter for them, with [[Class]] taken from Object.prototype.toString.call(object) for non-remote objects.

@janodvarko
janodvarko Feb 1, 2013

Good point, I made a note here:
https://getfirebug.com/wiki/index.php/JSD2_Adoption#Open_Tasks

Let's see what other requirements for this could yet appear.

Honza

for (var i = 0; i < reps.length; ++i)
{
View
2 extension/content/firebug/lib/css.js
@@ -295,7 +295,7 @@ Css.getElementCSSSelector = function(element)
label += "#" + element.id;
if (element.classList && element.classList.length > 0)
- label += "." + element.classList.item(0);
+ label += "." + element.classList[0];
return label;
};

0 comments on commit 5c220f5

Please sign in to comment.