The ESMP(ECMAScript Mobile Profile) is the OMA standard for Wireless markup scripting language known as ECMAScript Mobile Profile. It's strongly based upon ECMAScript Release 3 (ECMA262), While has some extensions and customizations . This bug report is to make JavascriptCore to support ESMP .
Created attachment 27913 [details] patch to enable ESMP support : MemoryError , and Error.code This is the first patch to enable ESMP support, this specifically enables a new native error type : MemoryError , and an new error property: Error.code. For more detail, please refer: Section 6.12.2 Native Error Types(Constants) of OMA-WAP-ESMP-V1_0-20050614-C
Created attachment 28069 [details] This patch is to throw URIError for invalid location URI This patch depends on the last patch. This patch is to throw URIError for invalid location URLs, to be compliance with ESMP (OMA-WAP-ESMP-V1_0-20050614-C) , section 9.4, Location Object.
Created attachment 28215 [details] add an optional parameter "bool caseSensitive" to Element.getAttribute(name), Element.setAttribute(name, value) and Element.removeAttribute(name) OMA-WAP-ESMP-V1_0-20050614-C has some extension to ECMA-262 , this patch includes the following extensions: Element.getAttribute(name) ==> Element.getAttribute(name, [Optional] caseSensitive) Element.setAttribute(name, value) ==>Element.setAttribute(name, value, [Optional] caseSensitive) Element.removeAttribute(name) ==> Element.removeAttribute(name, [Optional] caseSensitive)
*** Bug 23724 has been marked as a duplicate of this bug. ***
Created attachment 30500 [details] Updated patch for new ESMP error support As the replacer of Charles' patch(#27913), this patch adds some addtional messages into Charles' patch(id=27913), such as copyright information, bug# for tracing, etc
Created attachment 30501 [details] Updated patch for ESMP URIError handling This patch is a replacement for patch(id=28069), includs adding some copyright information, bug# information, and code style change
Created attachment 30502 [details] Updated patch for ESMP to support casecensitive attribute of element This patch is a replacement of the patch(id=28215), including some addtional code changes to get suitable for updated WebKit, adding copyright messages, bug# information, etc.
Comment on attachment 27913 [details] patch to enable ESMP support : MemoryError , and Error.code Please obsolete old patches when posting new ones.
(In reply to comment #8) > (From update of attachment 27913 [details] [review]) > Please obsolete old patches when posting new ones. Eric, I have no the permission to do it. thanks for your help.
Comment on attachment 30500 [details] Updated patch for new ESMP error support Minor style comment: +#if ENABLE(ESMP) + URIError = 6, + MemoryError = 7 +#else URIError = 6 +#endif It's ok to leave a trailing comma after the last enum element, so you don't need to duplicate URIError.I'm not a fan of the other code duplicate in the patch, but I don't see an easy way to avoid it. r- to address the minor style comment. Please repost a patch with that addressed.
(In reply to comment #10) > (From update of attachment 30500 [details] [review]) > Minor style comment: > +#if ENABLE(ESMP) > + URIError = 6, > + MemoryError = 7 > +#else > URIError = 6 > +#endif > It's ok to leave a trailing comma after the last enum element, so you don't > need to duplicate URIError.I'm not a fan of the other code duplicate in the > patch, but I don't see an easy way to avoid it. > r- to address the minor style comment. Please repost a patch with that > addressed. Alright, I will fix it soon. Thanks. :)
Comment on attachment 30501 [details] Updated patch for ESMP URIError handling Typo in the ChangeLog: + Through URIError for invalid URIs for location, for compliance with ESMP(OMA-WAP-ESMP-V1_0), This should say "Throw", not "Through". throwError(exec, URIError, (String("Invalid URI ") + dstUrl).utf8().data()); The .utf8().data() is not appropriate, this makes a temporary char*. Instead, you can just pass a String and rely on WebCore::String's implicit conversion to UString. This error appears in the code twice. Please address the above comments and repost.
Comment on attachment 30502 [details] Updated patch for ESMP to support casecensitive attribute of element It's unfortunate to do this by adding completely duplicate versions of removeAttribute/getAttribute/setAttribute. Perhaps at least some of these could just have the extra parameter and use of it as an #ifdef but share most of the body. Also, I don't understand why custom bindings are needed for getAttribute and removeAttribute. Why was that done? The ChangeLog should explain. Please address these comments and repost.
(In reply to comment #12) > (From update of attachment 30501 [details] [review]) > Typo in the ChangeLog: > + Through URIError for invalid URIs for location, for compliance with > ESMP(OMA-WAP-ESMP-V1_0), > This should say "Throw", not "Through". > throwError(exec, URIError, (String("Invalid URI ") + dstUrl).utf8().data()); > The .utf8().data() is not appropriate, this makes a temporary char*. Instead, > you can just pass a String and rely on WebCore::String's implicit conversion to > UString. This error appears in the code twice. > Please address the above comments and repost. ok, I will fix it later.
(In reply to comment #13) > (From update of attachment 30502 [details] [review]) > It's unfortunate to do this by adding completely duplicate versions of > removeAttribute/getAttribute/setAttribute. Perhaps at least some of these could > just have the extra parameter and use of it as an #ifdef but share most of the > body. > Also, I don't understand why custom bindings are needed for getAttribute and > removeAttribute. Why was that done? The ChangeLog should explain. > Please address these comments and repost. Without custom bindings for getAttribute/removeAttribute,we can't get or remove attributes according to the manner that ESMP defined through javascript. we need JSElement has the two functions binding with those of WebCore. Of course, code reuse should be considered as much as possible. I will fix it later. Thanks!
Created attachment 30648 [details] Latest patch for ESMP to support casesensitive attribute of element reconstructed the implmentation, and added more messages in Changelog
Created attachment 30649 [details] Latest patch to support addtional ESMP Errro type Addressed the comments of Maciej for patch 30500
Created attachment 30650 [details] Latest patch for ESMP URIError support updated as per Maciej's comments for patch 30501
Comment on attachment 30648 [details] Latest patch for ESMP to support casesensitive attribute of element I think the case sentitive support can be enabled in Element always, w/o guards. We don't get much from the guards: 5 #if ENABLE(ESMP) 56 PassRefPtr<Node> removeNamedItem(const String& name, ExceptionCode& code, bool caseSensitive = false); 57 #else 5458 PassRefPtr<Node> removeNamedItem(const String& name, ExceptionCode&); 59 #endif Basically I would remove the guards on all of the internal plumbing. The only thing which needs guards are the JS bindings. Why the copies of JSElment getAttribute and removeAttribute? Those don't seem needed. r- for the excessive use of #ifdef guards and the code duplication.
Comment on attachment 30649 [details] Latest patch to support addtional ESMP Errro type This is the wrong way to add code support: #if !ENABLE(ESMP) 247249 NativeErrorPrototype* evalErrorPrototype = new (exec) NativeErrorPrototype(exec, nativeErrorPrototypeStructure, "EvalError", "EvalError"); The current copy/paste approach using #ifdefs is very error-prone. Please instead add a mapping from the various error types to their codes. That mapping code would only be added in ESMP builds. Please add a LayoutTest to verify that the codes are properly exposed to JavaScript in ESMP builds. Please add a LayoutTest to test that memoryError exists. r- for lack of testing Some WebKit devs may disagree with me, but I think that it's better to have a few extra values (like the codes) in the common source base, than have copy/paste code to avoid them. :)
Comment on attachment 30650 [details] Latest patch for ESMP URIError support This needs LayoutTests.
Created attachment 41280 [details] patch for ESMP to support casesensitive attribute of element 1. Removed guards. 2. Added test case.
Created attachment 41281 [details] patch for ESMP to support casesensitive attribute of element Made some small modification.
Created attachment 41479 [details] patch for ESMP URIError support If enable ESMP, test case fast/dom/location-new-window-no-crash.html will faild. So, add it into ignore file list when ESMP is enabled.
Created attachment 41853 [details] patch for ESMP exception.code support
Comment on attachment 41853 [details] patch for ESMP exception.code support It's not right to use a single bug for all ESMP changes. We need to use separate bug report for each feature. > +#if ENABLE(ESMP) > + int code = 0; > + if (name == "RangeError") > + code = 101; > + else if (name == "ReferenceError") > + code = 102; > + else if (name == "SyntaxError") > + code = 103; > + else if (name == "TypeError") > + code = 104; > + else if (name == "URIError") > + code = 105; > + else if (name == "EvalError") > + code = 106; > + else if (name == "MemoryError") > + code = 99; > + putDirect(Identifier(exec, "code"), jsNumber(exec, code), 0); > +#endif This is an inefficient way to do this operation. Doing 7 string comparisons to accomplish this doesn't make sense. I suggest putting this into Error.cpp's Error::create function instead of doing what you're doing here. It seems quite strange to make ESMP a compile-time switch. Is this really the right way to do this? Who would use the ESMP version in what context? I'd like to know more before taking more of these patches.
(In reply to comment #26) > (From update of attachment 41853 [details]) > It's not right to use a single bug for all ESMP changes. We need to use > separate bug report for each feature. Personally, I think puting all patches for ESMP in one bug is convenient to track them. > > > +#if ENABLE(ESMP) > > + int code = 0; > > + if (name == "RangeError") > > + code = 101; > > + else if (name == "ReferenceError") > > + code = 102; > > + else if (name == "SyntaxError") > > + code = 103; > > + else if (name == "TypeError") > > + code = 104; > > + else if (name == "URIError") > > + code = 105; > > + else if (name == "EvalError") > > + code = 106; > > + else if (name == "MemoryError") > > + code = 99; > > + putDirect(Identifier(exec, "code"), jsNumber(exec, code), 0); > > +#endif > > This is an inefficient way to do this operation. Doing 7 string comparisons to > accomplish this doesn't make sense. I admit that this is not an efficient way, but considering this function is called very few times (called by JSGlobalObject::init() only when initializing JS runtime environment ), therefore, I think its efficiency is acceptable. > > I suggest putting this into Error.cpp's Error::create function instead of doing > what you're doing here. > Error::create() finally calls NativeErrorPrototype::NativeErrorPrototype() to constructe an error. We did this in your way in the previous patch, but Eric Seidel thought it's a wrong way. > It seems quite strange to make ESMP a compile-time switch. Is this really the > right way to do this? Who would use the ESMP version in what context? I'd like > to know more before taking more of these patches. ESMP support is added for peoples who want to port webkit to a phone, it is similar to WCSS and WML support.
Created attachment 41935 [details] patch for ESMP MemoryError support * Before testing, please change "sub hasESMPSupport" in "WebKitTools/Scripts/webkitdirs.pm" first: return 0; ==> return 1; * The test case is time-consuming.
Comment on attachment 41281 [details] patch for ESMP to support casesensitive attribute of element Following discussions at the TC39 meeting last week, ESMP is basically to be considered dead, so we won't accept ESMP patches to the main webkit tree
Comment on attachment 41479 [details] patch for ESMP URIError support Following discussions at the TC39 meeting last week, ESMP is basically to be considered dead, so we won't accept ESMP patches to the main webkit tree
Comment on attachment 41935 [details] patch for ESMP MemoryError support Following discussions at the TC39 meeting last week, ESMP is basically to be considered dead, so we won't accept ESMP patches to the main webkit tree
Following discussions at the TC39 meeting last week, ESMP is basically to be considered dead, so we won't accept ESMP patches to the main webkit tree For this reason I am closing out this bug as WONTFIX
(In reply to comment #32) > Following discussions at the TC39 meeting last week, ESMP is basically to be > considered dead, so we won't accept ESMP patches to the main webkit tree > > For this reason I am closing out this bug as WONTFIX While I don't necessarily disagree with the status, the reason for closing the bug is a different story. We're trying to determine now if ESMP is actually part of the hard requirements for certain setups. If so, it doesn't matter how declared dead it is, it's an engineering problem and needs to be solved. No different than replicating bugs in MS IE.
No comment, so reopen until we can verify that it isn't required for proxy compatibility.
This becomes invalid after 3 years opening.