Bugzilla – Full Text Bug Listing
| Summary: | Optimization of generated HTML file | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Will Smythe <smythew> | ||||||||||||||||
| Component: | EDT | Assignee: | Huang Ji Yong <hjiyong> | ||||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||
| Priority: | P3 | CC: | chenzhh, hjiyong, mayunf, svihovec, tww | ||||||||||||||||
| Version: | unspecified | ||||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||||
| Hardware: | All | ||||||||||||||||||
| OS: | All | ||||||||||||||||||
| Whiteboard: | RUI_Optimization | ||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||
| Bug Blocks: | 367818 | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
Created attachment 209210 [details]
Current (EDT 0.7) generated HTML file (for reference)
Created attachment 209242 [details]
Design
Created attachment 209243 [details]
My proposed html
Hi All,
I attach the design doc. Please review.
I discuss with Tony and Jimmy about the Will's proposed generated HTML. We agree with all of the design except the egl.require(handler) part.
Will's suggestion:
egl.require("samples.client.HelloWorldView"); // this will pull in all the .js this module depends on (Button.js, etc etc)
Our design:
egl.require("org.edt.widgets.button");
... // require all dependence
egl.require("sample.client.HelloWorldView");
The reason to make this change is that your design (let's call it modular approach) may need a lot of change in JSGen, JS runtime and HTML Gen while the change is transparent to most users (they don't modify the generated html) .
Our design only separate the js code from HTML file which makes little change to the previous proved model. And it also makes the HTML clean
Can we generate a file named <RUIHandlerName>_runtime.js, which contains the following content (See below). Then a user who wants to put a handler into their own HTML file would only need to import the one .js file and invoke a single function.
NOTE: I added a function named egl.<RUIHandler>.init() that in turn invokes egl.init with the appropriate information. The HTML file (generated or user defined) would be:
<script src = <RUIHandlerName>_runtime.js/>
<script >
egl.<RUIHandlerName>.init();
</script>
* We can discuss the need to fully qualify the name of the init function and runtime.js file to avoid collisions.
<RUIHandlerName>_runtime.js:
<noscript>JavaScript is currently not supported or enabled by this browser. Please enable JavaScript for full functionality. </noscript>
<script type="text/javascript" src="edt_core.js" />
<script type="text/javascript">
//Can we get the parameters from runtime?
egl.initParams = function("testweb", "hello", "en_US", "ENU", "MM/dd/yyyy", "MM-dd-yyyy", "MM-dd-yyyy", "$", ".", ",");
egl.require("egl.runtime");
egl.loadFile("config/includeDojo.html");
egl.require("dojo.widgets.DojoBase.js");
egl.require("dojo.widgets.DojoButton.js");
egl.require("samples.client.HelloWorldView");
egl.includeBind("hello");
egl.includeBind("org_eclipse_edt_rui_widgets_0_7_0");
egl.includeBind("org_eclipse_edt_rui_dojo_0_7_0");
egl.<RUIHandlerName>.init = function() {
egl.init("testweb", "en_US",
function() {
egl.rootHandler = new egl.samples.client.HelloWorldView();
egl.rootHandler.setParent(egl.Document);
},
function() {
egl.crashTerminateSession();
if (!egl.samples.client.HelloWorldView){
egl.println('Internal generation error. Found no definition for samples.client.HelloWorldView. Try <b>Project > Clean...</b>', e);
}else{ egl.printError('Could not render UI', e); throw e;}
}
);
}
</script>
Regarding the modular approach discussed in the design document, isn't this what we are already doing in development mode to support hot-swapping new types in the browser? I believe Yun Feng added this support? I believe a modular approach may also benefit Dynamic Loading, since we won't have to define a Service for determining which files are required by a part that is being loaded dynamically? In general, I am ok with cleaning up the HTML file in .8 I1 without introducing a modular solution, since cleaning things up will help us remove redundant code and make it easier to find and fix things. I would like to know what we are considering for I2 or I3 that may be more important than using that time to implement a modular solution. [NOTE: I am not entirely sure if I agree with a Modular solution at this time, but rejecting the idea because it will take too much time, without any qualification about why the issue may not make sense technically, does not seem like the right approach] Created attachment 209375 [details]
Patch to clean up the html
I attach the fix to implement my proposed design, please review. I think we can finish this in M1, prepare to do FVT and design the other issues later.
I suggest we could open another enhancement to discuss the modular loading problem if you agree.
I believe the patch in comment 7 doesn't take into account the changes in comment 5. Can you confirm this? Will and Ji Yong, can you provide your comments on the proposed changes listed in comment 5? I think we should commit the changes in comment 7 today, and then open a new enhancement to introduce modular loading for a future iteration. Before we close this enhancement, I would like to finish our discussion on the changes in comment 5. (In reply to comment #8) This fix does not include comment #5 (In reply to comment #5) > Can we generate a file named <RUIHandlerName>_runtime.js, which contains the > following content (See below). Then a user who wants to put a handler into > their own HTML file would only need to import the one .js file and invoke a > single function. > Your approach to generate one more js file may be a solution for the "open requirement" I think the approach still needs to refine 1. The content of your designed <RUIHandlerName>_runtime.js is actually an html file. A js file does not contain the <script> tag. 2. Running the init part in the js file will make the imported handler become the rootHandler. But that's too much because we just want to import the handler class into the page. Assume the your handler js file can work, I think the approach is another variation of the modular loading. It moves the dependency from the handler js file to another js file. To implement this approach also needs a lot of changes in the html generator frameworks (we now have to generate 2 files, need time to investigate) and need more testing effort. While the benefits is transparent for real users. Generally, I think we can discuss and compare this approach together with the modular approach. > //Can we get the parameters from runtime? > egl.initParams = function("testweb", "hello", "en_US", "ENU", > "MM/dd/yyyy", "MM-dd-yyyy", "MM-dd-yyyy", "$", ".", ","); Yes, we can get them in runtime (no implementation now), but may have different values because of the difference of platform (java and javascript). In current implementation, there will be no difference. (In reply to comment #6) > Regarding the modular approach discussed in the design document, isn't this > what we are already doing in development mode to support hot-swapping new types > in the browser? I believe Yun Feng added this support? Yes, the support is implemented in the development js generator which may indicate the approach is feasible. If we want to implement the approach, we have to move it to normal javascript generator, and may have to make change to the load function in runtime. Commit the patch in comment #7. Open a bug 368555 for the modular loading. Resolve this enhancement. Discuss for modular loading, please go to bug 368555, thanks. the HTML specification said <noscript> could not be in <head>. Need to move to <body> Some comments:
In egl_core.js. egl.getRuntimeMessage & the message files are not ready to be used. Need to replace that with more generic error reports.
if (!egl.newXMLHttpRequest) {
egl.printError(egl.getRuntimeMessage( "CRRUI2088E", []), null);
};
I noticed that changes was made to egl__contextKey policy so that if the HTML has contextKey, every js to be loaded will be appended the same contextKey. This should be changed. Please see Bug 368001 - Only use contextKey to load parts in the file being displayed in VE
egl.init taking two functions as parameters looks a bit complex to me, it also exposes unnecessary logic which can be hide in egl_core.js (I think). What about to make it only take 1 function, something like below. The key restriction here seemed to be 'only html knows the handler name, egl_core.js don't know that.
egl.init(
function(){
if (egl.client.Main){
egl.rootHandler = new egl.client.Main();
egl.rootHandler.setParent(egl.Document);
egl.startup();
}else{
egl.reportHandlerLoadError();
}
}
);
Created attachment 209554 [details]
Updated patch for Tony's comment
I've updated patch for comment 12 and the first problem of comment 13. Regarding the context key problem, I think we can discuss it in bug 368001. Regarding the egl.init function, I think the errorFunc still needs to be existed. The problem with the proposed approach is that, egl.client can be undefined, thus this function will throw exception immediately instead of going into "else" logic. if (egl.client.Main){ egl.rootHandler = new egl.client.Main(); egl.rootHandler.setParent(egl.Document); egl.startup(); }else{ egl.reportHandlerLoadError(); } I'd rather keep the old init function which contains an initFunc and errorFunc which is more natural, and can show user how to implement the error handling logic Created attachment 209620 [details] Updated fix for Tony's comment This fix covers comment 11, comment 12 problem 1 and 3. Problem 2 will be considered in the separate bug. The remaining topic to be discussed 1. We use egl.require("package.handler") rather than egl.require("package/handler.js") to load the dependent file. The "dot" representation looks good most of the time. But in some case it may look meaningless when the egl file name is a reserved word. For example, if egl handler file name is "abstract", the generated require statement will be egl.require("package.ezekw$$abastact"); //an alias name is generated for reserved words. The alias looks meaningless and cannot be hand written by developer. Which kind of representation shall we take? 2. Merge egl_core.js into egl.js and load egl.js at once. Brian mentioned this solution in an email thread. I think it is OK. The bootstrap js then may be bigger but since it will be loaded finally, it is ok to load it at the beginning. Commit the change for Tony's comment. close |
Created attachment 209209 [details] Mocked up new HTML file I think part of the 0.8 optimization work needs to involve cleaning up (i.e. optimizing) the HTML file generated for a RUI during deployment. The file is way too complicated. A good portion of it could be hidden away in a separate .js file (and I wonder whether some of it is even required). I took a crack at creating what I think a 'reasonable' generated HTML file might look like - see attached. Besides being much simpler, I think it demonstrates how a developer could easily embed a RUI component in an existing page (which is an open requirement). There is also the concept of a module (like Dojo has/had) that gets us away from including individual files, but instead including logical modules/packages, which may pull in .js, css, etc files. There is definitely some connection to dynamic loading here as well.