[Edbrowse-dev] patch remarks

[Edbrowse-dev] patch remarks

[Kevin Carhart]

[-- Attachment #1: Type: text/plain, Size: 9418 bytes --]

This is not the full code but some remarks on what and my reasoning.
I hope this email style is readable.





-----------------------------
Remark: This is the decorate.c portion
+set_property_string(io, "class", "");




---------------------------
Remark: I am going to redundantly establish both 'class' and 'className' in decorate because it seems that they are both used sometimes.
 



-----------------------
Remark: Here I redundantly add class as well as className to
the htmlclass
 	if (htmlclass) {
 		set_property_string(io, "className", htmlclass);
+		set_property_string(io, "class", htmlclass);
 	}
 






-----------------------

Remark: Now I'm in jsNode.  I specify nodeType in every case.
It is mostly set to 1, which is the value for any element.
Could this be done more generically - probably.
It may help clarity if it is spread out.
 
set_property_number(t->jv, "nodeType", 3);
set_property_number(t->jv, "nodeType", 1);
etc
etc
 
 
--------------------
Remark: Next change is that I commented out the "iframe" entry
from availableTags.  It seemed to crash edbrowse.  Pages do
exotic things inside of iframes.  So I wanted to see what would
happen if I set it aside and work without them temporarily.
@@ -1251,7 +1274,7 @@ const struct tagInfo availableTags[] = {
 	{"form", "a form", TAGACT_FORM, 10, 1},
 	{"button", "a button", TAGACT_INPUT, 0, 4},
 	{"frame", "a frame", TAGACT_FRAME, 2, 4},
-	{"iframe", "a frame", TAGACT_FRAME, 2, 4},
+//	{"iframe", "a frame", TAGACT_FRAME, 2, 4},




-------------------------
Remark: Next is the jseng-moz.cpp portion.  setTimeout is the only
function that I changed.
@@ -1888,12 +1888,17 @@ static JSObject *setTimeout(unsigned int argc, jsval * argv, bool isInterval)
 




------------------------
Remark: here are my setTimeout changes.  This is the only thing
I did to jseng-moz.cpp.  Is my solution leaky?  Could be.  If the 
callor sends legal code, I think it works.  If the callor doesn't
send legal code, you have problems other than the number of arguments.
It could use improvement.
-	if (argc != 2 || !JSVAL_IS_INT(argv[1]))
-		goto badarg;
 
 	v0 = argv[0];
-	v1 = argv[1];
-	n = JSVAL_TO_INT(v1);
+	if (argc != 2 || !JSVAL_IS_INT(argv[1]))
+	{
+		// if only one parameter was supplied, hardcode
+		// a number of milliseconds.  1?  100?  1000?
+		n = 100;
+	} else {
+        	v1 = argv[1];
+        	n = JSVAL_TO_INT(v1);
+	}
 	if (JSVAL_IS_STRING(v0) ||
 	    v0.isObject() &&
 	    JS_ValueToObject(jcx, v0, fo.address()) &&
@@ -1980,9 +1985,6 @@ abort:
 		return to;
 	}
 


---------------------
Remark: I entirely removed the badargs label
-badarg:
-	JS_ReportError(jcx, "invalid arguments to %s()", methname);
-	return NULL;
 }				/* setTimeout */
 



------------------
Remark: The rest of my changes are entirely in startwindow.js
-status = 0;
+// document.status is removed because it creates a conflict with
+// the status property of the XMLHttpRequest implementation
+// pages seem to want document.style to exist
+document.style = {"bgcolor":"white"};
 




-------------------
Remark: I think maybe both sides of the comparison need to be
lowercased in the three getElementsBy functions.  My example scenario
was that
var weird = document.getElementsByClassName("liveSmall toggled links")
returned no elements but
var weird = document.getElementsByClassName("livesmall toggled links")
did.
 document.gebtn$ = function(top, s) { 
 var a = new Array;
-if(s === '*' || (top.nodeName && top.nodeName === s))
+if(s === '*' || (top.nodeName && top.nodeName.toLowerCase() === s))
 a.push(top);
 if(top.childNodes) {
 for(var i=0; i<top.childNodes.length; ++i) {
@@ -81,7 +84,7 @@ return document.gebn$(this, s);
 }
 document.gebn$ = function(top, s) { 
 var a = new Array;
-if(s === '*' || (top.name && top.name === s))
+if(s === '*' || (top.name && top.name.toLowerCase() === s))
 a.push(top);
 if(top.childNodes) {
 for(var i=0; i<top.childNodes.length; ++i) {
@@ -98,7 +101,7 @@ return document.gebcn$(this, s);
 }
 document.gebcn$ = function(top, s) { 
 var a = new Array;
-if(s === '*' || (top.className && top.className === s))
+if(s === '*' || (top.className && top.className.toLowerCase() === s))
 a.push(top);
 if(top.childNodes) {
 for(var i=0; i<top.childNodes.length; ++i) {


---------------
Remark: I added a couple of functions to the URL prototype
so now it will have substr, substring, and also split


@@ -470,6 +476,10 @@ return this.toString().lastIndexOf(s);
 URL.prototype.substring = function(from, to) { 
 return this.toString().substring(from, to);
 }
+// pages expect both substring and substr 
+URL.prototype.substr = function(from, to) {
+return this.toString().substring(from, to);
+}
 URL.prototype.toLowerCase = function() { 
 return this.toString().toLowerCase();
 }
@@ -482,6 +492,9 @@ return this.toString().match(s);
 URL.prototype.replace = function(s, t) { 
 return this.toString().replace(s, t);
 }
+URL.prototype.split = function(s) {
+return this.toString().split(s);
+}
 
---------------------
Remark: here is the addEventListener change I mentioned.  I don't know
why it works, but it seems to work with it and not work without it.
I create an object called Event, and then pump it in as an argument
to the handler.  We might want to discuss this more.  Take the Event
away, and the onclick handler needed for the drescher scenario stops
working right.
The Event implementation is at the very end.






-------------------
Remark: The two changes shown for addEventListener are, first of all,
to preserve the event type before prepending "on" to it and then,
to alter the eval() call so that the newly instantiated Event of 
type click or whichever, can be pumped in as an argument.
 function addEventListener(ev, handler, notused)
 {
+ev_before_changes = ev;
 ev = "on" + ev;
 eval(
-'this.' + ev + ' = function(){ var a = this.' + evarray + '; if(this.' + evorig + ') this.' + evorig + '(); for(var i = 0; i<a.length; ++i) a[i](); };');
+'this.' + ev + ' = function(){ var a = this.' + evarray + '; if(this.' + evorig + ') this.' + evorig + '(); for(var i = 0; i<a.length; ++i) {var tempEvent = new Event;tempEvent.type = "' + ev_before_changes + '";a[i](tempEvent);} };');



---------------------
Remark: I implemented hasAttribute.  I added it to all of the prototype
sections.  I also added prototype sections for Image and Frame.
+// hasAttribute works in a comparable way to get and set, returning true/false
+document.hasAttribute = function(name) {
+if (this[name.toLowerCase()])
+        {
+                return true;
+        }
+        else
+        {
+                for (var i=0; i < this.attributes.length; ++i)
+                {
+                        if (this.attributes[i] == name.toLowerCase()) {
+                                return true;
+                        }
+                }
+        }
+return false;
+}
 
+
 
 
 
-----------------
Remark: I added some properties to the XMLHttpRequest implementation
for things that it is responsible for reporting, in case there is
a library involved.
 this.method = method || "GET";
 this.url = convert_url(url);
+this.status = 0;
+this.statusText = "";
 this.onreadystatechange();
 },
 setRequestHeader: function(header, value){
@@ -1165,6 +1249,8 @@ this.readyState = 4;
 
 if ((!this.aborted) && this.responseText.length > 0){
 this.readyState = 4;
+this.status = 200;
+this.statusText = "OK";
 this.onreadystatechange();
 }
 
 }
 




----------------------
Remark: Pages want some things having to do with CSS that we
haven't done yet, such as getComputedStyle, defaultView and
the CSS Specification.  So in the next change I add four things
having to do with how pages query objects for their styles.
+document.defaultView = function()
+{
+return this.style;
+}
+
+document.defaultView.getComputedStyle = function()
+{
+        obj = new CSSStyleDeclaration;
+        obj.element = document;
+        obj.style = document.style;
+        return obj;
+}
+
+getComputedStyle = function(n)
+{
+        obj = new CSSStyleDeclaration;
+        obj.element = this;
+        obj.style = new Array;
+        obj.style.push({n:obj.style[n]});
+        return obj;
+}
+
+CSSStyleDeclaration = function(){
+        this.element = null;
+        this.style = null;
+};
+
+CSSStyleDeclaration.prototype = {
+getPropertyValue: function (n)
+        {
+                if (this.style[n] == undefined)
+                {
+                        this.style[n] = 0;
+                        return 0;
+                } else {
+                        return this.style[n];
+                }
+        }
+}
+




-------------------
Remark: Last thing: this is my implementation of Event, which I added to the
addEventHandler code.




+Event = function(options){
+    this._bubbles = true;
+    this._cancelable = true;
+    this._cancelled = false;
+    this._currentTarget = null;
+    this._target = null;
+    this._eventPhase = Event.AT_TARGET;
+    this._timeStamp = new Date().getTime();
+    this._preventDefault = false;
+    this._stopPropagation = false;
+};
+
-- 
1.8.3.2

[Edbrowse-dev] patch remarks

[Karl Dahlke]

Well seems reasonable enough so I pushed it,
plus the substr correction you mentioned.

Commenting out iframe makes me a little nervous,
it fixes some pages that crashed but mightit also break some pages
that use to work??

I don't understand the new Event call, I guess it works,
but Event isn't a class it's a function, and has no return,
so what is happening with

tempEvent = new Event;

What is assigned to tempEvent?


Karl Dahlke

Re: [Edbrowse-dev] patch remarks

[Kevin Carhart]

> Well seems reasonable enough so I pushed it,
> plus the substr correction you mentioned.

Thanks Karl!

> Commenting out iframe makes me a little nervous,
> it fixes some pages that crashed but mightit also break some pages
> that use to work??

I know.  I agree... it might break some pages that used to work, so it 
probably should get uncommented again right away. 
It's like a working demo, which is now there for posterity in version 
control with a unique hash and timestamp, so it will be downloadable even 
when we supercede it with more commits.  Maybe another way 
of accomplishing this would have been to fork the edbrowse code so that it 
is sitting under kcarhart instead of CMB, commit the bundle of changes and 
say "would you please compile this in a side folder marked 'experiment' 
and try it out?  If I do these couple of weird things that are way out on 
a limb, as well as these other ten bits of implementation that I can 
justify, we can get this whole round trip where several moving parts of 
the program work together and a potentially common page behavior works 
which doesn't otherwise."  It's food for thought, in the form of a 
compileable codebase rather than just describing.  It kind of plays on the 
lovely fact that edbrowse is so light, you can copy around the entire 
thing just to illustrate a point, and it doesn't take prohibitive time to 
compile or download.

Kevin

[Edbrowse-dev] patch remarks

[Karl Dahlke]

Well yes you can make local versions and play with them,
but as for master I really want to just march along, come what may,
cause I don't think I'll ever understand branching and merging in git.
As you say, edbrowse is light, with not that many users
following the latest and greatest.
That's why we post stable versions.

Karl Dahlke