Wookie
  1. Wookie
  2. WOOKIE-44

Default widget returns js "error" when loaded into a browser

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.14.0
    • Component/s: Server
    • Labels:
      None
    • Environment:
      Windows Vista sp2, mysql 5.0, tomcat 5.5. (IE 8 & FF 3.5)

      Description

      Default widgets return js "error" when loaded into a browser

      (1) Navigate to http://localhost:8080/wookie
      (2) Click "instantiate a widget"
      (3) Use the default form data, click submit
      (4) Copy/paste the url found between the <url></url> tags into another browser window
      (5) Js error message appears

      This also happens for instances of the discussion & vote widgets

        Activity

        Hide
        Paul Sharples added a comment -

        Issue still affects IE8. No further reports since last release, moving down to 0.14.0.

        Show
        Paul Sharples added a comment - Issue still affects IE8. No further reports since last release, moving down to 0.14.0.
        Hide
        Paul Sharples added a comment -

        Moving to next release

        Show
        Paul Sharples added a comment - Moving to next release
        Hide
        Paul Sharples added a comment -

        We've had no reports or feedback regarding this issue. Moving down to next release, in case there are reports.

        Show
        Paul Sharples added a comment - We've had no reports or feedback regarding this issue. Moving down to next release, in case there are reports.
        Hide
        Paul Sharples added a comment -

        Issue refers to IE8. Moving down again to 0.10.1 but probably we should close this issue next time - we have had no reports/feedback form users stating that this is a problem

        Show
        Paul Sharples added a comment - Issue refers to IE8. Moving down again to 0.10.1 but probably we should close this issue next time - we have had no reports/feedback form users stating that this is a problem
        Hide
        Scott Wilson added a comment -

        This is an IE8 specific issue, with most users now hopefully using IE9. Moving down to next version to review again at a later date.

        Show
        Scott Wilson added a comment - This is an IE8 specific issue, with most users now hopefully using IE9. Moving down to next version to review again at a later date.
        Hide
        Paul Sharples added a comment -

        This is an IE8 specific issue, with most users now hopefully using IE9. Moving down to next version to review again at a later date.

        Show
        Paul Sharples added a comment - This is an IE8 specific issue, with most users now hopefully using IE9. Moving down to next version to review again at a later date.
        Hide
        Paul Sharples added a comment -

        Moving this to 0.9.2. Most IE users should now be using IE 9, but we can still leave it visible.

        Show
        Paul Sharples added a comment - Moving this to 0.9.2. Most IE users should now be using IE 9, but we can still leave it visible.
        Hide
        Paul Sharples added a comment -

        Moving this to 0.9.1, as although there is a workaround, this issue still needs to reviewed again

        Show
        Paul Sharples added a comment - Moving this to 0.9.1, as although there is a workaround, this issue still needs to reviewed again
        Hide
        Paul Sharples added a comment -

        This seems a sensible thing to do.

        Show
        Paul Sharples added a comment - This seems a sensible thing to do.
        Hide
        Ross Gardler added a comment -

        Should this really be closed? The problem appears to be specific to IE8. There is a warning displayed if the problem exists in a widget, but the issue itself is not resolved.

        I suggest we reopen this issue, but drop the priority and move it to a later milestone.

        My reasoning is to ensure that the issue remains visible and gets reviewed periodically. There may a better solution at some point in the future.

        Show
        Ross Gardler added a comment - Should this really be closed? The problem appears to be specific to IE8. There is a warning displayed if the problem exists in a widget, but the issue itself is not resolved. I suggest we reopen this issue, but drop the priority and move it to a later milestone. My reasoning is to ensure that the issue remains visible and gets reviewed periodically. There may a better solution at some point in the future.
        Hide
        Paul Sharples added a comment -

        I've also now commited some changes which do a compatibility check on the javascript code during the import process. It checks to see if the "widget.preferences.foo='bar'" syntax appears anywhere in js, or html files and displays a warning about the issue. It doesn't rewrite the code though, only flags a warning message.

        Show
        Paul Sharples added a comment - I've also now commited some changes which do a compatibility check on the javascript code during the import process. It checks to see if the "widget.preferences.foo='bar'" syntax appears anywhere in js, or html files and displays a warning about the issue. It doesn't rewrite the code though, only flags a warning message.
        Hide
        Paul Sharples added a comment -

        Original problem identified is now resolved

        Show
        Paul Sharples added a comment - Original problem identified is now resolved
        Hide
        Scott Wilson added a comment -

        Sounds like a good solution; I think its pretty likely anyone who already uses IE8 will migrate to IE9 when it appears.

        A compatibility check on import is an interesting idea, and could include other kinds of useful warnings - I suggest you close this issue and create a new feature for it.

        Show
        Scott Wilson added a comment - Sounds like a good solution; I think its pretty likely anyone who already uses IE8 will migrate to IE9 when it appears. A compatibility check on import is an interesting idea, and could include other kinds of useful warnings - I suggest you close this issue and create a new feature for it.
        Hide
        Paul Sharples added a comment -

        I've checked in a fix for IE9 beta which resolves the problem, but most IE users still will be on version 8 unfortunately.

        I tried to implement something to warn the user at runtime of the possible problem, in the 'wookie-wrapper.js file'. I did this by using the try/catch block to trap if the definition of the getter/setter failed.

        Widget.preferences.name=value; // is legal syntax so can't trap it at runtime

        However, I'm inclined not to do it this way as the warning would appear on each widget whether its using the syntax or not. Also If several widgets are loaded into a container application this could result in several alert boxes opening - not very nice. As it stands the wookie-wrapper.js will remain silent on the issue.

        I've added an entry for this on the FAQ page, if users want more information.

        As far as as doing a compatibilty check the only other thing I can think of is to parse every .js file on import of a widget zip file into the wookie server and try to replace any of the' Widget.preferences.foo=bar' type syntax with the 'Widget.preferences.setItem('foo','bar')' syntax.

        Show
        Paul Sharples added a comment - I've checked in a fix for IE9 beta which resolves the problem, but most IE users still will be on version 8 unfortunately. I tried to implement something to warn the user at runtime of the possible problem, in the 'wookie-wrapper.js file'. I did this by using the try/catch block to trap if the definition of the getter/setter failed. Widget.preferences.name=value; // is legal syntax so can't trap it at runtime However, I'm inclined not to do it this way as the warning would appear on each widget whether its using the syntax or not. Also If several widgets are loaded into a container application this could result in several alert boxes opening - not very nice. As it stands the wookie-wrapper.js will remain silent on the issue. I've added an entry for this on the FAQ page, if users want more information. As far as as doing a compatibilty check the only other thing I can think of is to parse every .js file on import of a widget zip file into the wookie server and try to replace any of the' Widget.preferences.foo=bar' type syntax with the 'Widget.preferences.setItem('foo','bar')' syntax.
        Hide
        Scott Wilson added a comment -

        The workaround is to always use:

        widget.preferences.setItem("foo", "bar");

        and never:

        widget.preferences.foo = "bar";

        ... which won't work in IE8 but will in FF, Safari and Opera.

        So its a case of checking the widget javascript to make sure it doesn't do it.

        However, I think its a reasonable approach to do IE8 compatibility checking on a test system before making a widget available in your main Wookie server; its highly unlikely this preference syntax thing individual widgets will make IE8 bork - for example some of the widgets I've done use HTML 5 features that aren't in IE.

        So maybe just add "uses only basic preferences method syntax" to your checklist when testing widgets.

        Meanwhile we wait and hope IE9 comes along and sorts this out ...

        Show
        Scott Wilson added a comment - The workaround is to always use: widget.preferences.setItem("foo", "bar"); and never: widget.preferences.foo = "bar"; ... which won't work in IE8 but will in FF, Safari and Opera. So its a case of checking the widget javascript to make sure it doesn't do it. However, I think its a reasonable approach to do IE8 compatibility checking on a test system before making a widget available in your main Wookie server; its highly unlikely this preference syntax thing individual widgets will make IE8 bork - for example some of the widgets I've done use HTML 5 features that aren't in IE. So maybe just add "uses only basic preferences method syntax" to your checklist when testing widgets. Meanwhile we wait and hope IE9 comes along and sorts this out ...
        Hide
        Steve Nisbet added a comment -

        Hi Paul,
        hope you are doing well, meant to get in touch sooner, but have actually
        been off over Easter. Just wanted to get a feel for where (as a
        community / development) we are up to with the IE prob in Wookie - as
        per the email below. Is there a proper workround now?

        Mark (Stubbs) is really keen on Wookie but we have a major commitment to
        IE8 in our standard platform build across the institution.

        You guys go to Moodle Moot?

        best wishes

        Steve


        Manchester Metropolitan University
        Learning Systems Manager
        LRT
        Manchester Metroopolitan University
        Before acting on this email or opening any attachments you should read the
        Manchester Metropolitan University's email disclaimer available on its website
        http://www.mmu.ac.uk/emaildisclaimer
        etc... and so on.

        Show
        Steve Nisbet added a comment - Hi Paul, hope you are doing well, meant to get in touch sooner, but have actually been off over Easter. Just wanted to get a feel for where (as a community / development) we are up to with the IE prob in Wookie - as per the email below. Is there a proper workround now? Mark (Stubbs) is really keen on Wookie but we have a major commitment to IE8 in our standard platform build across the institution. You guys go to Moodle Moot? best wishes Steve – Manchester Metropolitan University Learning Systems Manager LRT Manchester Metroopolitan University Before acting on this email or opening any attachments you should read the Manchester Metropolitan University's email disclaimer available on its website http://www.mmu.ac.uk/emaildisclaimer etc... and so on.
        Hide
        Paul Sharples added a comment -

        Widget.preferences.x = y and y= Widget.preferences.x

        ..should both work in IE8.

        But because we are directly assigning the values instead of using a setter, it is not calling the setItem() method.

        Show
        Paul Sharples added a comment - Widget.preferences.x = y and y= Widget.preferences.x ..should both work in IE8. But because we are directly assigning the values instead of using a setter, it is not calling the setItem() method.
        Hide
        Scott Wilson added a comment -

        So for IE8, you can only set/get a preference using:

        Widget.preferences.setItem(x,y) and Widget.preferences.getItem(x,y)

        But:

        Widget.preferences.x = y and y= Widget.preferences.x

        Will fail?

        If so I wonder if there is some better way of flagging this to developers; e.g. looking for this pattern in uploaded widget code and notifying the user that a widget won't work in IE8 because of this issue?

        We should certainly put an FAQ entry for it in any case.

        Show
        Scott Wilson added a comment - So for IE8, you can only set/get a preference using: Widget.preferences.setItem(x,y) and Widget.preferences.getItem(x,y) But: Widget.preferences.x = y and y= Widget.preferences.x Will fail? If so I wonder if there is some better way of flagging this to developers; e.g. looking for this pattern in uploaded widget code and notifying the user that a widget won't work in IE8 because of this issue? We should certainly put an FAQ entry for it in any case.
        Hide
        Paul Sharples added a comment -

        As Scott stated above, IE8 does not support _defineGetter_ and _defineSetter_ prototypes. Instead it uses the following syntax...

        Object.defineProperty(domObjectOnly + propName, {
        get: getMethod()

        {// code here}

        ,
        set: setMethod(param)

        { code here}

        });

        There appears to be no way to define setters/getters on user defined objects in IE. (only DOM objects). If you try to do it, you get the "unsupported method" alert. According to Microsoft, they intend to open up the Object.defineProperty mechanism to include user objects in IE9 (which is not until next summer unfortunately)

        I have checked in a partial fix for this and the error box now does not appear in IE8. I have used object assignment to get around the problem, so we can read the initial values into the preferences okay. However any updates to the preferences in the session are not persisted back to the wookie backend because the WidgetPreference.setItem() method is not being called and in turn neither is Widget.setPreferenceForKey(key, value);

        Show
        Paul Sharples added a comment - As Scott stated above, IE8 does not support _ defineGetter _ and _ defineSetter _ prototypes. Instead it uses the following syntax... Object.defineProperty(domObjectOnly + propName, { get: getMethod() {// code here} , set: setMethod(param) { code here} }); There appears to be no way to define setters/getters on user defined objects in IE. (only DOM objects). If you try to do it, you get the "unsupported method" alert. According to Microsoft, they intend to open up the Object.defineProperty mechanism to include user objects in IE9 (which is not until next summer unfortunately) I have checked in a partial fix for this and the error box now does not appear in IE8. I have used object assignment to get around the problem, so we can read the initial values into the preferences okay. However any updates to the preferences in the session are not persisted back to the wookie backend because the WidgetPreference.setItem() method is not being called and in turn neither is Widget.setPreferenceForKey(key, value);
        Hide
        Scott Wilson added a comment -

        The cause has been narrowed down to a problem with _defineGetter_ and _defineSetter_ prototype methods which have been removed from IE8 and replaced with the new defineProperty method; we'll have to detect IE8 and implement a special solution for it.

        See http://www.wdonline.com/blog/2009/01/14/ie8-interoperability/ for more info.

        Show
        Scott Wilson added a comment - The cause has been narrowed down to a problem with _ defineGetter _ and _ defineSetter _ prototype methods which have been removed from IE8 and replaced with the new defineProperty method; we'll have to detect IE8 and implement a special solution for it. See http://www.wdonline.com/blog/2009/01/14/ie8-interoperability/ for more info.
        Hide
        Paul Sharples added a comment - - edited

        Reason: Partial fix only
        --------------------------------
        The original js alert box which said "error" does not appear. Using Firefox 3.5.5 each of the widgets now appear to work correctly. (they didn't work originally). However, when using Internet Explorer 8 (on win vista), each of the widgets still display an error alert box on load, but the message is different...

        "Object doesn't support this property or method"

        How to reproduce...

        (1) Navigate to http://localhost:8080/wookie
        (2) Click "instantiate a widget"
        (3) Use the default form data, click submit
        (4) Copy/paste the url found between the <url></url> tags into another browser window
        (5) Js error message appears

        (Note: in firefox 3.5.5 the error does not appear at all)

        This happens for chat, forum, vote, natter & weather. (Note: After the message appears, the widgets appear to work okay in IE, except the weather widget)

        A clue might be in the local dwr adapter "wookie-wrapper.js" line 56 char 13 - as the weather widget falls over completely, pointing to this section of code. My guess is that IE is being fussy about some syntax here.

        Not sure if we should treat this as a new issue or not, but the fix is only partial, so I have rejected it.

        Show
        Paul Sharples added a comment - - edited Reason: Partial fix only -------------------------------- The original js alert box which said "error" does not appear. Using Firefox 3.5.5 each of the widgets now appear to work correctly. (they didn't work originally). However, when using Internet Explorer 8 (on win vista), each of the widgets still display an error alert box on load, but the message is different... "Object doesn't support this property or method" How to reproduce... (1) Navigate to http://localhost:8080/wookie (2) Click "instantiate a widget" (3) Use the default form data, click submit (4) Copy/paste the url found between the <url></url> tags into another browser window (5) Js error message appears (Note: in firefox 3.5.5 the error does not appear at all) This happens for chat, forum, vote, natter & weather. (Note: After the message appears, the widgets appear to work okay in IE, except the weather widget) A clue might be in the local dwr adapter "wookie-wrapper.js" line 56 char 13 - as the weather widget falls over completely, pointing to this section of code. My guess is that IE is being fussy about some syntax here. Not sure if we should treat this as a new issue or not, but the fix is only partial, so I have rejected it.
        Hide
        Scott Wilson added a comment -

        I've checked in a fix for this; turns out requests for shared data handled by the WidgetAPIImpl class were not adequately handling null responses.

        Show
        Scott Wilson added a comment - I've checked in a fix for this; turns out requests for shared data handled by the WidgetAPIImpl class were not adequately handling null responses.
        Hide
        Scott Wilson added a comment -

        Allocating to release 0.8.1

        Show
        Scott Wilson added a comment - Allocating to release 0.8.1
        Hide
        Scott Wilson added a comment -

        Yes, I can confirm this one.

        Show
        Scott Wilson added a comment - Yes, I can confirm this one.

          People

          • Assignee:
            Unassigned
            Reporter:
            Paul Sharples
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 0h
              0h
              Logged:
              Time Spent - 1h
              1h

                Development