Wookie
  1. Wookie
  2. WOOKIE-151

Numeric keys for preferences result in Parse Error in Safari

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.1
    • Component/s: None
    • Labels:
      None
    • Environment:
      Safari 5 Mac OS X

      Description

      If you create a widget that uses numeric keys for preference values, this can cause a Parse Error deeper in Wookie (most likely, in DWR) even if the values are cast to a String.

      To replicate, add this line to any widget:

      widget.preferences.setItem("12345","67890");

      Also:

      widget.preferences.setItem("12345a","67890");

      ...will fail, but not:

      widget.preferences.setItem("a12345","67890");

      I suspect this is something either in the wrapper.js or DWR engine.js. Unfortunately the debug tools in Safari aren't sufficient to trace it properly.

      As a workaround - just don't use numbers for preference keys!

        Activity

        Scott Wilson created issue -
        Hide
        Raido Kuli added a comment - - edited

        I've looked into this and found the line in wookie-wrapper which causes Syntax: parse error message.

        64: eval("Widget.preferences.prefs."key"=pref");

        It seems Safari can't handle evaluating for example "Widget.preferences.prefs.1245=somevalue".

        Edit:

        It's not actually Safari's problem, we should not allow passing numeric value for key or let it be widget developer's problem, because JavaScript actually doesn't allow numeric variables.

        Quote from W3C page:

        "Variable names must begin with a letter or the underscore character"

        Show
        Raido Kuli added a comment - - edited I've looked into this and found the line in wookie-wrapper which causes Syntax: parse error message. 64: eval("Widget.preferences.prefs." key "=pref"); It seems Safari can't handle evaluating for example "Widget.preferences.prefs.1245=somevalue". Edit: It's not actually Safari's problem, we should not allow passing numeric value for key or let it be widget developer's problem, because JavaScript actually doesn't allow numeric variables. Quote from W3C page: "Variable names must begin with a letter or the underscore character"
        Hide
        Scott Wilson added a comment -

        Great work Raido - I couldn't figure out where this was being thrown. That makes a lot of sense.

        Show
        Scott Wilson added a comment - Great work Raido - I couldn't figure out where this was being thrown. That makes a lot of sense.
        Hide
        Scott Wilson added a comment -

        I've flagged this up on the W3C list to see if there's any guidance on how we deal with this.

        Show
        Scott Wilson added a comment - I've flagged this up on the W3C list to see if there's any guidance on how we deal with this.
        Hide
        Scott Wilson added a comment -

        The advice I got on the W3C lists is to use array rather than dot notation to be on the safe side, e.g. widget.preferences[12345]=blah

        Show
        Scott Wilson added a comment - The advice I got on the W3C lists is to use array rather than dot notation to be on the safe side, e.g. widget.preferences [12345] =blah
        Raido Kuli made changes -
        Field Original Value New Value
        Assignee Raido Kuli [ raido357 ]
        Raido Kuli made changes -
        Status Ready To Review [ 10006 ] Open [ 1 ]
        Hide
        Raido Kuli added a comment -

        Implementing suggested solution works nice in Safari and Chrome, but IE 8 is dead and haven't tested on IE 9. Giving error "Unterminated constant string" 73: wookie_wrapper.js.

        73: eval("Widget.preferences." + key + "='" + value + "'"); <-- this is what i got from SVN - this breaks IE also.

        I tried it replacing with:

        eval("Widget.preferences[\"" + key + "\"]='" + value + "'");

        But obviously this Widget.preferences object doesn't handle data inserting like so.

        Commenting line 73 off, everything comes to life and no errors what so ever in IE.

        Another bug what i found while testing with IE is related to "seed-widget", which creates bad JS code. Seed-widget creates js/ui.js file, where on line 30 is a mistake.

        elemContentProperty.innerHTML = elemContent.innerHTML;

        has to be

        elemContentProperty.value = elemContent.innerHTML;

        Can't set textarea content using innerHTML - using "value" is the right way and works on Safari and IE - otherwise Safari gives blank textarea and IE throws error and dies.

        Show
        Raido Kuli added a comment - Implementing suggested solution works nice in Safari and Chrome, but IE 8 is dead and haven't tested on IE 9. Giving error "Unterminated constant string" 73: wookie_wrapper.js. 73: eval("Widget.preferences." + key + "='" + value + "'"); <-- this is what i got from SVN - this breaks IE also. I tried it replacing with: eval("Widget.preferences [\"" + key + "\"] ='" + value + "'"); But obviously this Widget.preferences object doesn't handle data inserting like so. Commenting line 73 off, everything comes to life and no errors what so ever in IE. Another bug what i found while testing with IE is related to "seed-widget", which creates bad JS code. Seed-widget creates js/ui.js file, where on line 30 is a mistake. elemContentProperty.innerHTML = elemContent.innerHTML; has to be elemContentProperty.value = elemContent.innerHTML; Can't set textarea content using innerHTML - using "value" is the right way and works on Safari and IE - otherwise Safari gives blank textarea and IE throws error and dies.
        Hide
        Raido Kuli added a comment -

        I think I've found solution for IE, by first just doing eval("Widget.preferences");

        And after that populating it with data:

        Widget.preferences[key] ...
        Widget.preferences.prefs[key]...

        I'll do some more testing on different browsers with this new code (array implementation and so on).

        Show
        Raido Kuli added a comment - I think I've found solution for IE, by first just doing eval("Widget.preferences"); And after that populating it with data: Widget.preferences [key] ... Widget.preferences.prefs [key] ... I'll do some more testing on different browsers with this new code (array implementation and so on).
        Scott Wilson made changes -
        Fix Version/s 0.9.1 [ 12315418 ]
        Hide
        Raido Kuli added a comment - - edited

        I've done some testing today, using numeric preference key's will cause DWR parse error.

        Modifying wookie-wrapper to use array-s works fine, but now DWR is causing problems.

        Used modified weather widget to test:

        changed preference key "city" to numeric value = fail.
        changed it to "a1234" for example, works.

        Produced errors:

        Safari - parse error
        Chrome - unexpected number
        IE / Firefox = Unexpected ;

        Show
        Raido Kuli added a comment - - edited I've done some testing today, using numeric preference key's will cause DWR parse error. Modifying wookie-wrapper to use array-s works fine, but now DWR is causing problems. Used modified weather widget to test: changed preference key "city" to numeric value = fail. changed it to "a1234" for example, works. Produced errors: Safari - parse error Chrome - unexpected number IE / Firefox = Unexpected ;
        Hide
        Scott Wilson added a comment -

        Thanks for keeping on with this Raido - DWR does seem to have some limitations.

        Show
        Scott Wilson added a comment - Thanks for keeping on with this Raido - DWR does seem to have some limitations.
        Hide
        Raido Kuli added a comment -

        I found the fix, i had missed some things in wookie-wrapper.js file.

        Tested on: IE8, Chrome, Safari, Opera - with weather widget using numeric key.

        Show
        Raido Kuli added a comment - I found the fix, i had missed some things in wookie-wrapper.js file. Tested on: IE8, Chrome, Safari, Opera - with weather widget using numeric key.
        Hide
        Scott Wilson added a comment -

        I gave it a try and it works great on Safari, Chrome and Firefox on Mac.

        Show
        Scott Wilson added a comment - I gave it a try and it works great on Safari, Chrome and Firefox on Mac.
        Hide
        Scott Wilson added a comment -

        Fixed by Raido

        Show
        Scott Wilson added a comment - Fixed by Raido
        Scott Wilson made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Hide
        Scott Wilson added a comment -

        Tested with lots of browsers.

        Show
        Scott Wilson added a comment - Tested with lots of browsers.
        Scott Wilson made changes -
        Status Closed [ 6 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            Raido Kuli
            Reporter:
            Scott Wilson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development