Last Comment Bug 616401 - nsHTMLCanvasElement::GetContext ignores JS exceptions
: nsHTMLCanvasElement::GetContext ignores JS exceptions
Status: RESOLVED FIXED
webgl-conformance
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: :Ms2ger (⌚ UTC+1/+2)
: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-02 22:27 PST by Luke Wagner [:luke]
Modified: 2012-05-18 06:33 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (5.11 KB, patch)
2012-05-05 08:06 PDT, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Patch v1 (diff -w) (3.34 KB, patch)
2012-05-05 08:06 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review

Description Luke Wagner [:luke] 2010-12-02 22:27:22 PST
nsHTMLCanvasElement::GetContext has a loop to the effect:

  for (...) {
    if (!JS_IdToValue(...) && !JS_GetPropertyById(...))
      continue;
    ...
  }

Thus, any exception thrown by JS_GetPropertyById (getter, invalid prop access) will be ignored or overwritten.  I'm no expert in the area, so maybe there is some higher-level design, perhaps some property of aContextOptions, that makes this not a problem.

Also, the return value of JS_ValueToString isn't tested for null.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2010-12-02 22:39:32 PST
Neither http://www.whatwg.org/specs/web-apps/current-work/complete/the-canvas-element.html#dom-canvas-getcontext nor https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#WEBGLCONTEXTATTRIBUTES seem to consider the possibility of exceptions here, and hence don't define behavior in those situations.  They probably need to do that.

Vlad, the webgl spec is probably where this should live....
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2012-01-20 11:50:34 PST
Blocking webgl-conformance so we can't forget this.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2012-05-05 08:06:05 PDT
Created attachment 621301 [details] [diff] [review]
Patch v1
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-05-05 08:06:49 PDT
Created attachment 621302 [details] [diff] [review]
Patch v1 (diff -w)
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2012-05-08 21:47:16 PDT
Comment on attachment 621301 [details] [diff] [review]
Patch v1

diff -w would have been nice....

r=me
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2012-05-18 02:51:44 PDT
https://hg.mozilla.org/mozilla-central/rev/09df0008b156

(In reply to Boris Zbarsky (:bz) from comment #5)
> Comment on attachment 621301 [details] [diff] [review]
> Patch v1
> 
> diff -w would have been nice....
> 
> r=me

Er, attachment 621302 [details] [diff] [review]?
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2012-05-18 06:33:18 PDT
Er, indeed....

Note You need to log in before you can comment on or make changes to this bug.