Re: [Wikiwyg-dev] Re: BUG: JavaScript strict warnings (Dave Howorth)
Grrrrrr. I feel... no, I *am* stupid. I was never subscribed to this list. And
all this time I thought nobody cared... %sigh%
Well, here I am and with a lot of catching up to do...
Alex,
I have a lot of questions on this patch. Some of these questions might be
addressed in earlier posts that I have not read yet (grrrrr), but here I go
anyway:
As far as I know, these 3 function headers mean the same thing (in the global
scope):
onload = function() { ...
window.onload = function() { ...
function onload() { ...
and even this:
this.onload = function() { ...
So this:
window.onload = function onload() { ...
seems redundant at best, and this:
proto.insert_text_at_cursor = function insert_text_at_cursor(text) {
just seems wrong.
---
Next, variables defined at the global scope are global whether or not you use
`var`. So the Wikiwyg style is to not use `var` there:
-klass = Wikiwyg.Wikitext;
+var klass = Wikiwyg.Wikitext;
---
Also I'm not sure why you /do/ return null sometimes and don't others:
+ return null; // Silence strict warning.
vs:
- return false
+ return;
---
If you could address these for me I surely appreciate it.
A final note... this patch contains quite a few different types of
changes. It is better for me to have more smaller patches than one big
patch. Of course that's a little bit harder for you since you don't have
commit access yet.
Cheers, Ingy
On 16/02/06 09:48 -0800, Alex Vincent wrote:
> Here's the first-round patch. I left two strict warnings unfixed
> because I wasn't sure what precisely caused them, and it's dangerous to
> make assumptions without fully understanding the code.
>
> JavaScript strict warning:
> http://demo.wikiwyg.net/wikiwyg/lib/Wikiwyg.js, line 157: reference to
> undefined property config[mode_object.classtype]
>
> http://demo.wikiwyg.net/wikiwyg/lib/Wikiwyg/Wikitext.js, line 935:
> reference to undefined property attributes[i]
>
> Index: wikiwyg-remote/demo/configure/index.html
> ===================================================================
> --- wikiwyg-remote/demo/configure/index.html (revision 8151)
> +++ wikiwyg-remote/demo/configure/index.html (working copy)
> @@ -7,7 +7,7 @@
> <script type="text/javascript" src="../../lib/Wikiwyg/Wikitext.js"></script>
> <script type="text/javascript" src="../../lib/Wikiwyg/Preview.js"></script>
> <script>
> -window.onload = function() {
> +window.onload = function onload() {
> var div1 = document.getElementById('example1');
> var config1 = {
> doubleClickToEdit: false,
> @@ -82,7 +82,7 @@
> <blockquote>
> <pre>
> <script>
> -window.onload = function() {
> +window.onload = function onload() {
> var div = document.getElementById('example1');
> var config = {
> doubleClickToEdit: false,
> @@ -137,7 +137,7 @@
> <blockquote>
> <pre>
> <script>
> -window.onload = function() {
> +window.onload = function onload() {
> var div2 = document.getElementById('example2');
> var config2 = {
> doubleClickToEdit: true,
> Index: wikiwyg-remote/demo/html/index.html
> ===================================================================
> --- wikiwyg-remote/demo/html/index.html (revision 8151)
> +++ wikiwyg-remote/demo/html/index.html (working copy)
> @@ -11,7 +11,7 @@
> <script>
> // Find all the divs in the page (after it has loaded) and Wikiwygify them.
> var wikiwyg_divs = [];
> -onload = function() {
> +window.onload = function onload() {
> config = {
> imagesLocation: '../../images/',
> doubleClickToEdit: true,
> Index: wikiwyg-remote/demo/clientserver/index.html
> ===================================================================
> --- wikiwyg-remote/demo/clientserver/index.html (revision 8151)
> +++ wikiwyg-remote/demo/clientserver/index.html (working copy)
> @@ -10,7 +10,7 @@
> <script>
> // Find all the divs in the page (after it has loaded) and Wikiwygify them.
> var wikiwyg_divs = [];
> -onload = function() {
> +window.onload = function onload() {
> var config = {
> doubleClickToEdit: true,
> toolbar: {
> Index: wikiwyg-remote/demo/standalone/index.html
> ===================================================================
> --- wikiwyg-remote/demo/standalone/index.html (revision 8151)
> +++ wikiwyg-remote/demo/standalone/index.html (working copy)
> @@ -10,7 +10,7 @@
> <script>
> // Find all the divs in the page (after it has loaded) and Wikiwygify them.
> var wikiwyg_divs = [];
> -onload = function() {
> +window.onload = function onload() {
> var elements = document.getElementsByTagName('div');
> var divs = [];
> for (var i = 0; i < elements.length; i++)
> Index: wikiwyg-remote/lib/Wikiwyg.js
> ===================================================================
> --- wikiwyg-remote/lib/Wikiwyg.js (revision 8151)
> +++ wikiwyg-remote/lib/Wikiwyg.js (working copy)
> @@ -48,7 +48,7 @@
> Subclass - this can be used to create new classes
> =============================================================================*/
>
> -Subclass = function(name, base) {
> +var Subclass = function(name, base) {
> if (!name) die("Can't create a subclass without a name");
>
> var parts = name.split('.');
> @@ -89,6 +89,7 @@
> "baseclass was: " + this.baseclass + "\n" +
> "caller was: " + arguments.callee.caller
> );
> + return null; // Silence strict warning.
> }
> }
>
> @@ -97,7 +98,7 @@
> =============================================================================*/
>
> // Constructor and class methods
> -proto = new Subclass('Wikiwyg');
> +var proto = new Subclass('Wikiwyg');
>
> Wikiwyg.VERSION = '0.12';
>
> @@ -354,7 +355,7 @@
> return elem;
> }
>
> -die = function(e) { // See IE, below
> +var die = function(e) { // See IE, below
> throw(e);
> }
>
> Index: wikiwyg-remote/lib/Wikiwyg/Wikitext.js
> ===================================================================
> --- wikiwyg-remote/lib/Wikiwyg/Wikitext.js (revision 8151)
> +++ wikiwyg-remote/lib/Wikiwyg/Wikitext.js (working copy)
> @@ -25,7 +25,7 @@
> =============================================================================*/
>
> proto = new Subclass('Wikiwyg.Wikitext', 'Wikiwyg.Mode');
> -klass = Wikiwyg.Wikitext;
> +var klass = Wikiwyg.Wikitext;
>
> proto.classtype = 'wikitext';
> proto.modeDescription = 'Wikitext';
> @@ -307,7 +307,7 @@
> return string;
> }
>
> -proto.insert_text_at_cursor = function(text) {
> +proto.insert_text_at_cursor = function insert_text_at_cursor(text) {
> var t = this.area;
>
> var selection_start = t.selectionStart;
> @@ -316,7 +316,7 @@
> if (selection_start == null) {
> selection_start = selection_end;
> if (selection_start == null) {
> - return false
> + return;
> }
> }
>
> @@ -379,8 +379,11 @@
> if (markup_start != ' ')
> this.sel = this.sel.replace(/^ */gm, '');
> }
> - // if some other style, switch to new style
> - else if (match = this.sel.match(other_markup_re))
> + /* if some other style, switch to new style
> + * double parentheses to make it clear we're assigning match,
> + * and then testing it.
> + */
> + else if ((match = this.sel.match(other_markup_re)))
> // if pre, just indent
> if (markup_start == ' ')
> this.sel = this.sel.replace(/^/gm, markup_start);
> @@ -423,7 +426,10 @@
> this.sel = this.sel.replace(already_start, '');
> this.sel = this.sel.replace(already_finish, '');
> }
> - else if (match = this.sel.match(other_start)) {
> + /* double parentheses to make it clear we're assigning match,
> + * and then testing it.
> + */
> + else if ((match = this.sel.match(other_start))) {
> this.sel = this.sel.replace(other_start, markup_start);
> this.sel = this.sel.replace(other_finish, markup_finish);
> }
> @@ -711,15 +717,17 @@
> return true;
> }
>
> -proto.getFirstTextNode = function(element) {
> - for (node = element; node && node.nodeType != 3; node = node.firstChild) {
> - }
> +proto.getFirstTextNode = function getFirstTextNode(element) {
> + var node = element.firstChild;
> + while (node && node.nodeType != 3)
> + node = node.firstChild;
> return node;
> }
>
> -proto.getLastTextNode = function(element) {
> - for (node = element; node && node.nodeType != 3; node = node.lastChild) {
> - }
> +proto.getLastTextNode = function getLastTextNode(element) {
> + var node = element.lastChild;
> + while (node && node.nodeType != 3)
> + node = node.lastChild;
> return node;
> }
>
> @@ -927,7 +935,7 @@
> this.walk(element);
> }
>
> -proto.format_span = function(element) {
> +proto.format_span = function format_span(element) {
> if (this.is_opaque(element)) {
> this.handle_opaque_phrase(element);
> return;
> Index: wikiwyg-remote/lib/Wikiwyg/Wysiwyg.js
> ===================================================================
> --- wikiwyg-remote/lib/Wikiwyg/Wysiwyg.js (revision 8151)
> +++ wikiwyg-remote/lib/Wikiwyg/Wysiwyg.js (working copy)
> @@ -123,7 +123,7 @@
> this.get_edit_document().body.innerHTML = html;
> }
>
> -proto.apply_stylesheets = function(styles) {
> +proto.apply_stylesheets = function apply_stylesheets() {
> var styles = document.styleSheets;
> var head = this.get_edit_document().getElementsByTagName("head")[0];
>
> @@ -229,19 +229,20 @@
>
> proto.do_unlink = proto.exec_command;
>
> -proto.do_link = function() {
> +proto.do_link = function do_link() {
> var selection = this.get_link_selection_text();
> if (! selection) return;
> var url;
> var match = selection.match(/(.*?)\b((?:http|https|ftp|irc|file):\/\/\S+)(.*)/);
> if (match) {
> - if (match[1] || match[3]) return null;
> + if (match[1] || match[3]) return;
> url = match[2];
> }
> else {
> url = '?' + escape(selection);
> }
> this.exec_command('createlink', url);
> + return;
> }
>
> proto.get_selection_text = function() { // See IE, below
> @@ -252,7 +253,7 @@
> var selection = this.get_selection_text();
> if (! selection) {
> alert("Please select the text you would like to turn into a link.");
> - return;
> + return null;
> }
> return selection;
> }
> _______________________________________________
> Wikiwyg-dev mailing list
> Wikiwyg-dev@wikiwyg.net
> http://wikiwyg.net/mailman/listinfo/wikiwyg-dev
This archive was generated by a fusion of
Pipermail (Mailman edition) and
MHonArc.