Closed Bug 1325995 Opened 3 years ago Closed 3 years ago

Failure in 2.0.0/conformance2/glsl3/shader-with-invalid-characters.html

Categories

(Core :: Canvas: WebGL, defect, P3, trivial)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 --- wontfix
firefox53 --- fixed

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 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 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 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 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 on attachment 8823207 [details]
Bug 1325995 - Disallow backslash in WebGL 1. -

https://reviewboard.mozilla.org/r/101790/#review102394
Attachment #8823207 - Flags: review?(dmu) → review+
You need to log in before you can comment on or make changes to this bug.