Currently, WebSocket extensions which WebKit implement are all enabled by default. But this enabled extension list could be changed when WebKit support other extensions or rename existing extensions. It means that the expectation of a test which verify extensions property depends on WebKit. This is not good for embedder's unit tests. So, I'd like to add a layout test where server pyewebsocket just omit all extensions and the extensions property returns empty string.
Created attachment 130937 [details] Patch
Comment on attachment 130937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130937&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:25 > + shouldBeEqualToString("ws.extensions", "x-webkit-deflate-frame"); shouldBeEqualToString("ws.extensions", ""); ?
Comment on attachment 130937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130937&action=review >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:25 >> + shouldBeEqualToString("ws.extensions", "x-webkit-deflate-frame"); > > shouldBeEqualToString("ws.extensions", ""); ? Oops, right. I mistakenly overwrite the expectation.
Created attachment 130940 [details] Patch
Comment on attachment 130940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130940&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension-expected.txt:7 > +PASS ws.extensions is "" > +PASS ws.extensions is "" > +PASS ws.extensions is "" It's hard to understand what are tested here with this result. At least we should show: Check the value before opening the connection: PASS ws.extensions is "" PASS ws.extensions = "foo"; ws.extensions is "" Check the value after opening the connection: PASS ws.extensions is "" > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 > +ws.extensions = "foo"; > +shouldBeEqualToString("ws.extensions", ""); I prefer shouldBeEqualToString("ws.extensions = 'foo'; ws.extensions", ""); > LayoutTests/http/tests/websocket/tests/hybi/extensions-expected.txt:7 > PASS ws.extensions is "" > PASS ws.extensions is "" > +PASS ws.extensions is "x-webkit-deflate-frame" I'd like to make the same comment with echo-with-no-extension-expected.txt.
Comment on attachment 130940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130940&action=review >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension-expected.txt:7 >> +PASS ws.extensions is "" > > It's hard to understand what are tested here with this result. At least we should show: > > Check the value before opening the connection: > PASS ws.extensions is "" > PASS ws.extensions = "foo"; ws.extensions is "" > Check the value after opening the connection: > PASS ws.extensions is "" Agreed. >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 >> +shouldBeEqualToString("ws.extensions", ""); > > I prefer shouldBeEqualToString("ws.extensions = 'foo'; ws.extensions", ""); I see. >> LayoutTests/http/tests/websocket/tests/hybi/extensions-expected.txt:7 >> +PASS ws.extensions is "x-webkit-deflate-frame" > > I'd like to make the same comment with echo-with-no-extension-expected.txt. Done.
Created attachment 130975 [details] Patch
Comment on attachment 130975 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130975&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 > +// extensions attribute is read-only. > +debug("Check if the value is read only:"); nit: The comment is not helpful. > LayoutTests/http/tests/websocket/tests/hybi/extensions.html:20 > // extensions attribute is read-only. ditto.
Comment on attachment 130975 [details] Patch Clearing flags on attachment: 130975 Committed r110274: <http://trac.webkit.org/changeset/110274>
All reviewed patches have been landed. Closing bug.
This revision (r110274) causes fail on Qt. The diff: --- /ramdisk/qt-linux-64-release/build/layout-test-results/http/tests/websocket/tests/hybi/extensions-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/http/tests/websocket/tests/hybi/extensions-actual.txt @@ -7,7 +7,7 @@ Check if the value is read only: PASS ws.extensions = 'foo'; ws.extensions is "" Check the value after the connection is established: -PASS ws.extensions is "x-webkit-deflate-frame" +FAIL ws.extensions should be x-webkit-deflate-frame. Was . PASS successfullyParsed is true TEST COMPLETE
(In reply to comment #11) > This revision (r110274) causes fail on Qt. The diff: It's the same reason with compressed-control-frame.html, deflate-frame-invalid-parameter.html, deflate-frame-parameter.html, and handshake-fail-by-extensions-header.html. They are skipped in Qt.
(In reply to comment #12) > It's the same reason with compressed-control-frame.html, deflate-frame-invalid-parameter.html, deflate-frame-parameter.html, and handshake-fail-by-extensions-header.html. They are skipped in Qt. Thank you for the information, test will be inserted to that group.