According to WebGL's revision 10, extensions should be named: WEBGL_lose_context, WEBGL_depth_texture, WEBGL_compressed_texture_s3tc.
Created attachment 204975 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_
Comment on attachment 204975 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:-2414 > - || equalIgnoringCase(name, "WEBKIT_WEBGL_lose_context") Is it safe to remove these checks?? Seems blink still has the checks in place.
(In reply to comment #2) > (From update of attachment 204975 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:-2414 > > - || equalIgnoringCase(name, "WEBKIT_WEBGL_lose_context") > > Is it safe to remove these checks?? Seems blink still has the checks in place. It is clear that it may break some backward-compatibility in web applications that use this extension with WEBKIT_WEBGL_ prefix. However latest official specification from Khronos http://www.khronos.org/registry/webgl/extensions/ does not mention any WEBKIT_ extensions anymore. The code should be kept clean and compliant with spec all the time and this backward-compatibility, should be removed. If not now, then when?
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 204975 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > The code should be kept clean and compliant with spec all the time and this backward-compatibility, should be removed. If not now, then when? I am sure there is a process in place to handle these kind of issues in WebKit. It's just that I am not so familiar with the process. From the comments it clearly states that we are keeping them around for a certain grace period.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 204975 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > > The code should be kept clean and compliant with spec all the time and this backward-compatibility, should be removed. If not now, then when? > > I am sure there is a process in place to handle these kind of issues in WebKit. It's just that I am not so familiar with the process. From the comments it clearly states that we are keeping them around for a certain grace period. These comments were made on december 2011 (almost 2 years ago). Isn't the grace period long enough? What process exactly are you talking about? Do you know anyone we should consult about this?
(In reply to comment #2) > (From update of attachment 204975 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:-2414 > > - || equalIgnoringCase(name, "WEBKIT_WEBGL_lose_context") > > Is it safe to remove these checks?? Seems blink still has the checks in place. Hey, i dug recently into blink/chrom code, and found out, that they are not using WEBKIT_ prefix anymore: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLLoseContext.cpp&q=WEBGL_lose_context&sq=package:chromium&type=cs&l=79 https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLDepthTexture.cpp&q=WEBGL_depth_texture&sq=package:chromium&type=cs https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLCompressedTextureS3TC.cpp&q=WEBGL_compressed_texture_s3tc&sq=package:chromium&type=cs
(In reply to comment #6) > (In reply to comment #2) > > (From update of attachment 204975 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > > > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:-2414 > > > - || equalIgnoringCase(name, "WEBKIT_WEBGL_lose_context") > > > > Is it safe to remove these checks?? Seems blink still has the checks in place. > > Hey, i dug recently into blink/chrom code, and found out, that they are not using WEBKIT_ prefix anymore: > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLLoseContext.cpp&q=WEBGL_lose_context&sq=package:chromium&type=cs&l=79 > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLDepthTexture.cpp&q=WEBGL_depth_texture&sq=package:chromium&type=cs > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/canvas/WebGLCompressedTextureS3TC.cpp&q=WEBGL_compressed_texture_s3tc&sq=package:chromium&type=cs This is inaccurate I believe, take a look at WebGLRenderingContext::ExtensionTracker::matchesNameWithPrefixes(). The code was merely refactored.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (In reply to comment #2) > > > > (From update of attachment 204975 [details] [details] [details] [details]) > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > > > The code should be kept clean and compliant with spec all the time and this backward-compatibility, should be removed. If not now, then when? > > > > I am sure there is a process in place to handle these kind of issues in WebKit. It's just that I am not so familiar with the process. From the comments it clearly states that we are keeping them around for a certain grace period. > > These comments were made on december 2011 (almost 2 years ago). Isn't the grace period long enough? > What process exactly are you talking about? Do you know anyone we should consult about this? As said, I am not familiar with how we should decide to do these kind of changes (which effect backward compatibility etc). Sending a mail to webkit mailing list might help here.
(In reply to comment #8) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > (In reply to comment #2) > > > > > (From update of attachment 204975 [details] [details] [details] [details] [details]) > > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > As said, I am not familiar with how we should decide to do these kind of changes (which effect backward compatibility etc). Sending a mail to webkit mailing list might help here. LGTM. https://lists.webkit.org/pipermail/webkit-dev/2013-July/025127.html
Comment on attachment 204975 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ Thanks!
Comment on attachment 204975 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ Rejecting attachment 204975 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 204975, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/994753
(In reply to comment #10) > (From update of attachment 204975 [details]) > Thanks! hi dean, commit-bot says there is OOPS! (no reviewer put in changelog) can i put you name there?
Comment on attachment 204975 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ View in context: https://bugs.webkit.org/attachment.cgi?id=204975&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by (OOPS!! NOBODY). It is because this line is badly formatted. Add Dean's name here and reupload. Then request only cq, not review.
Created attachment 206371 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ the old one didn't have reviewer put inside attached new patch
Comment on attachment 206371 [details] Change recently ratified extensions prefixes from WEBKIT_WEBGL_ to WEBGL_ Clearing flags on attachment: 206371 Committed r152527: <http://trac.webkit.org/changeset/152527>
All reviewed patches have been landed. Closing bug.