Closed
Bug 1325995
Opened 7 years ago
Closed 7 years ago
Failure in 2.0.0/conformance2/glsl3/shader-with-invalid-characters.html
Categories
(Core :: Graphics: CanvasWebGL, defect, P3)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: jgilbert, Assigned: jgilbert)
Details
(Whiteboard: gfx-noted)
Attachments
(4 files)
When we strip comments, we don't handle line-continuation for single-line comments.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8822803 [details] Bug 1325995 - Rewrite shader comment truncation. - https://reviewboard.mozilla.org/r/101582/#review102164 ::: dom/canvas/WebGLValidateStrings.cpp:86 (Diff revision 1) > + NS_LITERAL_STRING("\n"), > + nsString() }; > + const nsString blockCommentEndings[] = { NS_LITERAL_STRING("\n"), > + NS_LITERAL_STRING("*/"), > + nsString() }; > I don't know why you all add nsString() at the arrays? It should be fine if removing them. ::: dom/canvas/WebGLValidateStrings.cpp:120 (Diff revision 1) > - // to preserve column numbers for debugging purposes. > - break; > + break; > - } > + } > -} > + } > > -/****** END CODE TAKEN FROM WEBKIT ******/ > + MOZ_ASSERT((dstBegin+1) - dstBegin == 1); I believe this is always right. Why you wannt add this?
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8822804 [details] Bug 1325995 - Do our own GLSL char validity checks based on ESSL3 rules. - https://reviewboard.mozilla.org/r/101584/#review102174 LGTM
Attachment #8822804 -
Flags: review?(dmu) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
status-firefox51:
--- → wontfix
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8822803 [details] Bug 1325995 - Rewrite shader comment truncation. - https://reviewboard.mozilla.org/r/101582/#review102164 > I don't know why you all add nsString() at the arrays? It should be fine if removing them. It's so we can skip the 'found' string unconditionally. (index of not-found would yield "") I added comments to make this clearer. > I believe this is always right. Why you wannt add this? It clarifies the intent here. Yes, it's always true, but it's hard to see whether the intent here is to get the byte count or character count, which are different here.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8822803 [details] Bug 1325995 - Rewrite shader comment truncation. - https://reviewboard.mozilla.org/r/101582/#review102384 r=me after adding some comments to describe why having nsString() in the comment checking arrays.
Attachment #8822803 -
Flags: review?(dmu) → review+
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8823207 [details] Bug 1325995 - Disallow backslash in WebGL 1. - https://reviewboard.mozilla.org/r/101790/#review102394
Attachment #8823207 -
Flags: review?(dmu) → review+
Comment 10•7 years ago
|
||
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4e24c140961 Rewrite shader comment truncation. - r=daoshengmu https://hg.mozilla.org/integration/mozilla-inbound/rev/704fc596dcb1 Do our own GLSL char validity checks based on ESSL3 rules. - r=daoshengmu https://hg.mozilla.org/integration/mozilla-inbound/rev/65260b9e453c Update tests. https://hg.mozilla.org/integration/mozilla-inbound/rev/2b0e3933ece4 Disallow backslash in WebGL 1. - r=daoshengmu https://hg.mozilla.org/integration/mozilla-inbound/rev/9307d60e21ba Misc fixes.
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b4e24c140961 https://hg.mozilla.org/mozilla-central/rev/704fc596dcb1 https://hg.mozilla.org/mozilla-central/rev/65260b9e453c https://hg.mozilla.org/mozilla-central/rev/2b0e3933ece4 https://hg.mozilla.org/mozilla-central/rev/9307d60e21ba
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•