Opened 14 years ago

Closed 12 years ago

#385 closed defect (fixed)

fixing the javascript code with missing ; for crunching

Reported by: Kim Steinhaug Owned by: gogo
Priority: normal Milestone: Version 1.0
Component: Xinha Core Version: trunk
Severity: normal Keywords: crunch, javascript, optimize, benchmark, compress


When trying to crunch the JS files one of the first problems arising is the missing ; on many of the function in the htmlarea.js file.

If you look at this one :

HTMLArea.onload = function(){};

This is correctly ended with a ; but if you look at functions like :

  • HTMLArea.prototype.registerPanel
  • HTMLArea.Config.prototype.addToolbarElement
  • And many more...

Its missing, meaning a crucnher will fail atonce unless someone manually crunches one and one function, rather timeconsuming since the problem is regarding inconcistency in the JS file.

Anyone with committing rights please jump in on this one.

Change History (15)

comment:1 Changed 14 years ago by mokhet

I'll look at it in a few days if nobody else has jumped on it, it's holidays for 3 more days :D

comment:2 Changed 14 years ago by anonymous

Sounds great, I will be expermineting with different crunchers soon and post my findings. The more automatic the crunching can be done the better, as of now crunching the main .js file must be done quite manual and takes alot of time, which means one looses interest in either crunching or updating the core on a regular basis.

Enjoy your holidays mokhet

comment:3 Changed 14 years ago by gogo

Kim: email me if you want SVN commit access - james@…

comment:4 Changed 14 years ago by mokhet

A lot of them (in htlmarea.js) has been fixed in changeset[341], i'm looking for more in the others files

comment:5 Changed 14 years ago by Kim Steinhaug

Here is a good place to validate the JS code aswell, abit problematic since its very strict in some comparison situations with NULL and chokes in some regexes - i do not want to mess with regexes, :D

comment:6 Changed 14 years ago by mokhet

should a basic function be ended by a ; or not ?

function foo()

I mean, indeed it's working with and without, but even if i never add a ; after thoses declarations, perhaps it's needed. What are you thinking ?

comment:7 Changed 14 years ago by mokhet

more fixes in changeset:376

comment:8 Changed 14 years ago by mokhet

and more in changeset:377

Let's hope we will finally track down all of them :)

comment:9 Changed 13 years ago by gogo

  • Milestone set to Version 1.0

should a basic function be ended by a ; or not ?

No, you do not need a ; after a closing brace.

comment:10 Changed 13 years ago by mokhet

more ; fixed (a few were missing, a lot were used with no reason) in changeset:419

comment:11 Changed 13 years ago by mokhet

More fixes to htmlarea.js in changeset:472

Fixed items :

  • missing braces
  • missing ;
  • variables declaration in wrong scope
  • duplicated variables declaration
  • usage variables before declaration
  • unnecessary blocks especially in case statements
  • coercition type comparaison (use === and !== instead of == and != ) for 0, "", false, true and null
  • a few fix of strict line ending
  • regular expression syntax to avoid ambiguity (as stated in jslint doc)
  • Use the object literal notation {} instead of new Object();
  • Use the array literal notation [] instead of new Array();
  • undefined variables

jslint is still complaining for usage of eval, which is evil, but i'm sorry i cant touch thoses parts of code yet.

It is also complaining about strict line ending, needs to check the checkbox "Lax line ending" to complete the process, sorry again but some of the tests are really hard to update to make jslint accept them.

And last one, it is also worried about a "Problem at line 3329 character 44: Expected a 'break' statement before 'case'.", this is due to complexity of the switch code used. I think we should rewrite a little the function _formatBlock() to avoid this code weirdness.

comment:12 Changed 13 years ago by mokhet

making htmlarea.js compliant to BSD style coding in changeset:473 has removed all the jslint complaint about stric line ending.

comment:13 Changed 13 years ago by mokhet

popupwin.js fixed in changeset:475

comment:14 Changed 12 years ago by mokhet

as stated in #899, #385 and #899 are duplicate. Sinec #899 is closed, we may closed this one too.

comment:15 Changed 12 years ago by gogo

  • Resolution set to fixed
  • Status changed from new to closed

Closing this. See also #905

Note: See TracTickets for help on using tickets.