Skip to content

Commit

Permalink
Widget: Throw errors when calling non-existent methods or methods on …
Browse files Browse the repository at this point in the history
…uninistantiated widgets. Fixes #5972 - Widget: Throw error for non-existent method calls.
  • Loading branch information
scottgonzalez committed Aug 27, 2010
1 parent 52a052b commit 1e28040
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 38 deletions.
3 changes: 0 additions & 3 deletions tests/unit/accordion/accordion_methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ test("init", function() {
$('<div></div>').appendTo('body').remove().accordion().remove();
ok(true, '.accordion() called on disconnected DOMElement - removed');

$('<div></div>').accordion().accordion("foo").remove();
ok(true, 'arbitrary method called after init');

var el = $('<div></div>').accordion();
var foo = el.accordion("option", "foo");
el.remove();
Expand Down
17 changes: 0 additions & 17 deletions tests/unit/dialog/dialog_core.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,6 @@ function margin(el, side) {

module("dialog: core");

test("element types", function() {
var typeNames = ('p,h1,h2,h3,h4,h5,h6,blockquote,ol,ul,dl,div,form'
+ ',table,fieldset,address,ins,del,em,strong,q,cite,dfn,abbr'
+ ',acronym,code,samp,kbd,var,img,object,hr'
+ ',input,button,label,select,iframe').split(',');

$.each(typeNames, function(i) {
var typeName = typeNames[i];
el = $(document.createElement(typeName)).appendTo('body');
(typeName == 'table' && el.append("<tr><td>content</td></tr>"));
el.dialog();
ok(true, '$("&lt;' + typeName + '/&gt").dialog()');
el.dialog("destroy");
el.remove();
});
});

test("title id", function() {
expect(3);

Expand Down
8 changes: 1 addition & 7 deletions tests/unit/dialog/dialog_methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module("dialog: methods", {
});

test("init", function() {
expect(7);
expect(6);

$("<div></div>").appendTo('body').dialog().remove();
ok(true, '.dialog() called on element');
Expand All @@ -24,9 +24,6 @@ test("init", function() {
$('<div></div>').appendTo('body').remove().dialog().remove();
ok(true, '.dialog() called on disconnected DOMElement - removed');

$('<div></div>').dialog().dialog("foo").remove();
ok(true, 'arbitrary method called after init');

el = $('<div></div>').dialog();
var foo = el.dialog("option", "foo");
el.remove();
Expand All @@ -46,9 +43,6 @@ test("destroy", function() {
$('<div></div>').dialog().dialog("destroy").remove();
ok(true, '.dialog("destroy") called on disconnected DOMElement');

$('<div></div>').dialog().dialog("destroy").dialog("foo").remove();
ok(true, 'arbitrary method called after destroy');

var expected = $('<div></div>').dialog(),
actual = expected.dialog('destroy');
equals(actual, expected, 'destroy is chainable');
Expand Down
1 change: 1 addition & 0 deletions tests/unit/slider/slider.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<link rel="stylesheet" href="../../../external/qunit.css" type="text/css"/>
<script type="text/javascript" src="../../../external/qunit.js"></script>
<script type="text/javascript" src="../../jquery.simulate.js"></script>
<script type="text/javascript" src="../testsuite.js"></script>

<script type="text/javascript" src="slider_core.js"></script>
<script type="text/javascript" src="slider_defaults.js"></script>
Expand Down
8 changes: 1 addition & 7 deletions tests/unit/slider/slider_methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
module("slider: methods");

test("init", function() {
expect(6);
expect(5);

$("<div></div>").appendTo('body').slider().remove();
ok(true, '.slider() called on element');
Expand All @@ -17,9 +17,6 @@ test("init", function() {
$('<div></div>').slider().remove();
ok(true, '.slider() called on disconnected DOMElement');

$('<div></div>').slider().slider("foo").remove();
ok(true, 'arbitrary method called after init');

var el = $('<div></div>').slider();
var foo = el.slider("option", "foo");
el.remove();
Expand All @@ -39,9 +36,6 @@ test("destroy", function() {
$('<div></div>').appendTo('body').remove().slider().slider("destroy").remove();
ok(true, '.slider("destroy") called on disconnected DOMElement');

$('<div></div>').slider().slider("destroy").slider("foo").remove();
ok(true, 'arbitrary method called after destroy');

var expected = $('<div></div>').slider(),
actual = expected.slider('destroy');
equals(actual, expected, 'destroy is chainable');
Expand Down
13 changes: 9 additions & 4 deletions ui/jquery.ui.widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,15 @@ $.widget.bridge = function( name, object ) {

if ( isMethodCall ) {
this.each(function() {
var instance = $.data( this, name ),
methodValue = instance && $.isFunction( instance[options] ) ?
instance[ options ].apply( instance, args ) :
instance;
var instance = $.data( this, name );
if ( !instance ) {
throw "cannot call methods on " + name + " prior to initialization; " +
"attempted to call method '" + options + "'";
}
if ( !$.isFunction( instance[options] ) ) {
throw "no such method '" + options + "' for " + name + " widget instance";
}
var methodValue = instance[ options ].apply( instance, args );
if ( methodValue !== instance && methodValue !== undefined ) {
returnValue = methodValue;
return false;
Expand Down

2 comments on commit 1e28040

@jzaefferer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be even more useful to do throw new Error("[message]") - that way the exception will contain fileName and lineNumber properties, making debugging even easier.

A quick Firebug test seems to confirm that, but I'm actually not sure about the exact semantics of throw and Error...

@scottgonzalez
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this for consistency with core. We could make this change and see about getting it into core as well.

Please sign in to comment.