148531 2015-08-27 10:32:54 -0700 Document window.NodeFilter properties 2015-08-28 20:20:21 -0700 1 1 1 Unclassified WebKit DOM Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=148602 P2 Normal --- 148415 1 cdumez cdumez bfulgham commit-queue darin dbates ggaren kling rniwa sam oldest_to_newest 1121346 0 cdumez 2015-08-27 10:32:54 -0700 Document window.NodeFilter properties to detect changes to them. This is in preparation of Bug 148415 as this will help detect unwanted changes to the interface when making NodeFilter a callback interface. 1121349 1 260068 cdumez 2015-08-27 10:36:20 -0700 Created attachment 260068 Patch 1121437 2 260068 ggaren 2015-08-27 14:39:04 -0700 Comment on attachment 260068 Patch r=me 1121441 3 cdumez 2015-08-27 14:42:44 -0700 Committed r189059: <http://trac.webkit.org/changeset/189059> 1121711 4 bfulgham 2015-08-28 09:19:28 -0700 This produces an interesting test failure on Windows: --- /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/node-filter-interface-expected.txt +++ /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/node-filter-interface-actual.txt @@ -10,7 +10,7 @@ * FILTER_SKIP [Configurable: false, enumerable: true, writable: false, value: 3] * SHOW_ALL -[Configurable: false, enumerable: true, writable: false, value: 4294967295] +[Configurable: false, enumerable: true, writable: false, value: -1] * SHOW_ATTRIBUTE [Configurable: false, enumerable: true, writable: false, value: 2] * SHOW_CDATA_SECTION It looks like Windows is using an unsigned value for whatever flag is being displayed here. Windows uses a signed value for enums by default -- is this flag an enum? I might need to make sure the enum is typed as unsigned so the Windows build works properly. 1121722 5 cdumez 2015-08-28 09:31:48 -0700 (In reply to comment #4) > This produces an interesting test failure on Windows: > > --- > /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/ > node-filter-interface-expected.txt > +++ > /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/ > node-filter-interface-actual.txt > @@ -10,7 +10,7 @@ > * FILTER_SKIP > [Configurable: false, enumerable: true, writable: false, value: 3] > * SHOW_ALL > -[Configurable: false, enumerable: true, writable: false, value: 4294967295] > +[Configurable: false, enumerable: true, writable: false, value: -1] > * SHOW_ATTRIBUTE > [Configurable: false, enumerable: true, writable: false, value: 2] > * SHOW_CDATA_SECTION > > It looks like Windows is using an unsigned value for whatever flag is being > displayed here. Windows uses a signed value for enums by default -- is this > flag an enum? > > I might need to make sure the enum is typed as unsigned so the Windows build > works properly. Yes, it looks like a bad bug if NodeFilter.SHOW_ALL maps to -1 on Windows. It is supposed to be 0xFFFFFFFF as per the spec. It looks like we may need to tweak the following enum in NodeFilter.h to do the right thing on Windows as well: enum { SHOW_ALL = 0xFFFFFFFF, SHOW_ELEMENT = 0x00000001, SHOW_ATTRIBUTE = 0x00000002, SHOW_TEXT = 0x00000004, SHOW_CDATA_SECTION = 0x00000008, SHOW_ENTITY_REFERENCE = 0x00000010, SHOW_ENTITY = 0x00000020, SHOW_PROCESSING_INSTRUCTION = 0x00000040, SHOW_COMMENT = 0x00000080, SHOW_DOCUMENT = 0x00000100, SHOW_DOCUMENT_TYPE = 0x00000200, SHOW_DOCUMENT_FRAGMENT = 0x00000400, SHOW_NOTATION = 0x00000800 }; 1121723 6 cdumez 2015-08-28 09:33:23 -0700 (In reply to comment #5) > (In reply to comment #4) > > This produces an interesting test failure on Windows: > > > > --- > > /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/ > > node-filter-interface-expected.txt > > +++ > > /home/buildbot/slave/win-release-tests/build/layout-test-results/fast/dom/ > > node-filter-interface-actual.txt > > @@ -10,7 +10,7 @@ > > * FILTER_SKIP > > [Configurable: false, enumerable: true, writable: false, value: 3] > > * SHOW_ALL > > -[Configurable: false, enumerable: true, writable: false, value: 4294967295] > > +[Configurable: false, enumerable: true, writable: false, value: -1] > > * SHOW_ATTRIBUTE > > [Configurable: false, enumerable: true, writable: false, value: 2] > > * SHOW_CDATA_SECTION > > > > It looks like Windows is using an unsigned value for whatever flag is being > > displayed here. Windows uses a signed value for enums by default -- is this > > flag an enum? > > > > I might need to make sure the enum is typed as unsigned so the Windows build > > works properly. > > Yes, it looks like a bad bug if NodeFilter.SHOW_ALL maps to -1 on Windows. > It is supposed to be 0xFFFFFFFF as per the spec. It looks like we may need > to tweak the following enum in NodeFilter.h to do the right thing on Windows > as well: > enum { > SHOW_ALL = 0xFFFFFFFF, > SHOW_ELEMENT = 0x00000001, > SHOW_ATTRIBUTE = 0x00000002, > SHOW_TEXT = 0x00000004, > SHOW_CDATA_SECTION = 0x00000008, > SHOW_ENTITY_REFERENCE = 0x00000010, > SHOW_ENTITY = 0x00000020, > SHOW_PROCESSING_INSTRUCTION = 0x00000040, > SHOW_COMMENT = 0x00000080, > SHOW_DOCUMENT = 0x00000100, > SHOW_DOCUMENT_TYPE = 0x00000200, > SHOW_DOCUMENT_FRAGMENT = 0x00000400, > SHOW_NOTATION = 0x00000800 > }; Are you able to try something like this? diff --git a/Source/WebCore/dom/NodeFilter.h b/Source/WebCore/dom/NodeFilter.h index 26f7663..061129a 100644 --- a/Source/WebCore/dom/NodeFilter.h +++ b/Source/WebCore/dom/NodeFilter.h @@ -53,7 +53,7 @@ public: * their values are derived by using a bit position corresponding * to the value of NodeType for the equivalent node type. */ - enum { + enum : unsigned long { SHOW_ALL = 0xFFFFFFFF, SHOW_ELEMENT = 0x00000001, SHOW_ATTRIBUTE = 0x00000002, 260068 2015-08-27 10:36:20 -0700 2015-08-27 14:40:58 -0700 Patch bug-148531-20150827103608.patch text/plain 4186 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg5MDI4CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNDQyODE5MDQxMjFmM2MyZGQ2YzQz NGY1YjQ3ZjJjOGU3NGNiNTU2Ni4uMjlhN2E5MjdlYmI5MDMzZTUxYTIxMjNjYWQyMTQyNDVjNjhk NWM5YiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE1LTA4LTI3ICBDaHJpcyBEdW1leiAgPGNk dW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgRG9jdW1lbnQgd2luZG93Lk5vZGVGaWx0ZXIgcHJv cGVydGllcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTQ4NTMxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg RG9jdW1lbnQgd2luZG93Lk5vZGVGaWx0ZXIgcHJvcGVydGllcyB0byBkZXRlY3QgY2hhbmdlcyB0 byB0aGVtLiBUaGlzCisgICAgICAgIGlzIGluIHByZXBhcmF0aW9uIG9mIEJ1ZyAxNDg0MTUgYXMg dGhpcyB3aWxsIGhlbHAgZGV0ZWN0IHVud2FudGVkCisgICAgICAgIGNoYW5nZXMgdG8gdGhlIGlu dGVyZmFjZSB3aGVuIG1ha2luZyBOb2RlRmlsdGVyIGEgY2FsbGJhY2sgaW50ZXJmYWNlLgorCisg ICAgICAgICogZmFzdC9kb20vbm9kZS1maWx0ZXItaW50ZXJmYWNlLWV4cGVjdGVkLnR4dDogQWRk ZWQuCisgICAgICAgICogZmFzdC9kb20vbm9kZS1maWx0ZXItaW50ZXJmYWNlLmh0bWw6IEFkZGVk LgorCiAyMDE1LTA4LTI3ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgogCiAg ICAgICAgIEFkZCBhbiBleHBlY3RhdGlvbiBmb3IgYW5vdGhlciBtb21lbnR1bSBzY3JvbGxpbmcg dGVzdCB0aGF0IGJlY2FtZSBmbGFreS4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvZG9t L25vZGUtZmlsdGVyLWludGVyZmFjZS1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9mYXN0L2Rv bS9ub2RlLWZpbHRlci1pbnRlcmZhY2UtZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0 CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLjc1ODE2NDcw YjRhYWQyNDVkMzA5ZjgxMmE1MTcyNjNkNDMwYjRlNDcKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlv dXRUZXN0cy9mYXN0L2RvbS9ub2RlLWZpbHRlci1pbnRlcmZhY2UtZXhwZWN0ZWQudHh0CkBAIC0w LDAgKzEsNDcgQEAKK0xpc3QgTm9kZUZpbHRlcidzIHByb3BlcnRpZXMgdG8gZGV0ZWN0IGNoYW5n ZXMgbWFrZSB0byBpdC4KKworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQ QVNTIiBtZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKKyogRklMVEVS X0FDQ0VQVAorW0NvbmZpZ3VyYWJsZTogZmFsc2UsIGVudW1lcmFibGU6IHRydWUsIHdyaXRhYmxl OiBmYWxzZSwgdmFsdWU6IDFdCisqIEZJTFRFUl9SRUpFQ1QKK1tDb25maWd1cmFibGU6IGZhbHNl LCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0YWJsZTogZmFsc2UsIHZhbHVlOiAyXQorKiBGSUxURVJf U0tJUAorW0NvbmZpZ3VyYWJsZTogZmFsc2UsIGVudW1lcmFibGU6IHRydWUsIHdyaXRhYmxlOiBm YWxzZSwgdmFsdWU6IDNdCisqIFNIT1dfQUxMCitbQ29uZmlndXJhYmxlOiBmYWxzZSwgZW51bWVy YWJsZTogdHJ1ZSwgd3JpdGFibGU6IGZhbHNlLCB2YWx1ZTogNDI5NDk2NzI5NV0KKyogU0hPV19B VFRSSUJVVEUKK1tDb25maWd1cmFibGU6IGZhbHNlLCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0YWJs ZTogZmFsc2UsIHZhbHVlOiAyXQorKiBTSE9XX0NEQVRBX1NFQ1RJT04KK1tDb25maWd1cmFibGU6 IGZhbHNlLCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0YWJsZTogZmFsc2UsIHZhbHVlOiA4XQorKiBT SE9XX0NPTU1FTlQKK1tDb25maWd1cmFibGU6IGZhbHNlLCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0 YWJsZTogZmFsc2UsIHZhbHVlOiAxMjhdCisqIFNIT1dfRE9DVU1FTlQKK1tDb25maWd1cmFibGU6 IGZhbHNlLCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0YWJsZTogZmFsc2UsIHZhbHVlOiAyNTZdCisq IFNIT1dfRE9DVU1FTlRfRlJBR01FTlQKK1tDb25maWd1cmFibGU6IGZhbHNlLCBlbnVtZXJhYmxl OiB0cnVlLCB3cml0YWJsZTogZmFsc2UsIHZhbHVlOiAxMDI0XQorKiBTSE9XX0RPQ1VNRU5UX1RZ UEUKK1tDb25maWd1cmFibGU6IGZhbHNlLCBlbnVtZXJhYmxlOiB0cnVlLCB3cml0YWJsZTogZmFs c2UsIHZhbHVlOiA1MTJdCisqIFNIT1dfRUxFTUVOVAorW0NvbmZpZ3VyYWJsZTogZmFsc2UsIGVu dW1lcmFibGU6IHRydWUsIHdyaXRhYmxlOiBmYWxzZSwgdmFsdWU6IDFdCisqIFNIT1dfRU5USVRZ CitbQ29uZmlndXJhYmxlOiBmYWxzZSwgZW51bWVyYWJsZTogdHJ1ZSwgd3JpdGFibGU6IGZhbHNl LCB2YWx1ZTogMzJdCisqIFNIT1dfRU5USVRZX1JFRkVSRU5DRQorW0NvbmZpZ3VyYWJsZTogZmFs c2UsIGVudW1lcmFibGU6IHRydWUsIHdyaXRhYmxlOiBmYWxzZSwgdmFsdWU6IDE2XQorKiBTSE9X X05PVEFUSU9OCitbQ29uZmlndXJhYmxlOiBmYWxzZSwgZW51bWVyYWJsZTogdHJ1ZSwgd3JpdGFi bGU6IGZhbHNlLCB2YWx1ZTogMjA0OF0KKyogU0hPV19QUk9DRVNTSU5HX0lOU1RSVUNUSU9OCitb Q29uZmlndXJhYmxlOiBmYWxzZSwgZW51bWVyYWJsZTogdHJ1ZSwgd3JpdGFibGU6IGZhbHNlLCB2 YWx1ZTogNjRdCisqIFNIT1dfVEVYVAorW0NvbmZpZ3VyYWJsZTogZmFsc2UsIGVudW1lcmFibGU6 IHRydWUsIHdyaXRhYmxlOiBmYWxzZSwgdmFsdWU6IDRdCisqIGxlbmd0aAorW0NvbmZpZ3VyYWJs ZTogdHJ1ZSwgZW51bWVyYWJsZTogZmFsc2UsIHdyaXRhYmxlOiBmYWxzZSwgdmFsdWU6IDBdCisq IG5hbWUKK1tDb25maWd1cmFibGU6IHRydWUsIGVudW1lcmFibGU6IGZhbHNlLCB3cml0YWJsZTog ZmFsc2UsIHZhbHVlOiBOb2RlRmlsdGVyXQorKiBwcm90b3R5cGUKK1tDb25maWd1cmFibGU6IGZh bHNlLCBlbnVtZXJhYmxlOiBmYWxzZSwgd3JpdGFibGU6IGZhbHNlLCB2YWx1ZTogW29iamVjdCBO b2RlRmlsdGVyUHJvdG90eXBlXV0KK1BBU1Mgc3VjY2Vzc2Z1bGx5UGFyc2VkIGlzIHRydWUKKwor VEVTVCBDT01QTEVURQorCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9mYXN0L2RvbS9ub2RlLWZp bHRlci1pbnRlcmZhY2UuaHRtbCBiL0xheW91dFRlc3RzL2Zhc3QvZG9tL25vZGUtZmlsdGVyLWlu dGVyZmFjZS5odG1sCm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLjM1YjYyM2FiOGUxZjEwMWYyMzZiZjI2YjVhZjQ3NDcw YTVmNzJlYzUKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2RvbS9ub2RlLWZp bHRlci1pbnRlcmZhY2UuaHRtbApAQCAtMCwwICsxLDIwIEBACis8IURPQ1RZUEUgaHRtbD4KKzxo dG1sPgorPGJvZHk+Cis8c2NyaXB0IHNyYz0iLi4vLi4vcmVzb3VyY2VzL2pzLXRlc3QtcHJlLmpz Ij48L3NjcmlwdD4KKzxzY3JpcHQ+CitkZXNjcmlwdGlvbigiTGlzdCBOb2RlRmlsdGVyJ3MgcHJv cGVydGllcyB0byBkZXRlY3QgY2hhbmdlcyBtYWtlIHRvIGl0LiIpOworCitmdW5jdGlvbiBsb2dQ cm9wZXJ0eURlc2NyaXB0b3IoZGVzY3JpcHRvcikgeworICAgIGRlYnVnKCJbQ29uZmlndXJhYmxl OiAiICsgZGVzY3JpcHRvci5jb25maWd1cmFibGUgKyAiLCBlbnVtZXJhYmxlOiAiICsgZGVzY3Jp cHRvci5lbnVtZXJhYmxlICsgIiwgd3JpdGFibGU6ICIgKyBkZXNjcmlwdG9yLndyaXRhYmxlICsg IiwgdmFsdWU6ICIgKyBkZXNjcmlwdG9yLnZhbHVlICsgIl0iKTsKK30KKwordmFyIHByb3BlcnRp ZXMgPSBPYmplY3QuZ2V0T3duUHJvcGVydHlOYW1lcyhOb2RlRmlsdGVyKS5zb3J0KCk7Citmb3Ig KHZhciBpID0gMDsgaSA8IHByb3BlcnRpZXMubGVuZ3RoOyBpKyspIHsKKyAgICBkZWJ1ZygiKiAi ICsgcHJvcGVydGllc1tpXSk7CisgICAgbG9nUHJvcGVydHlEZXNjcmlwdG9yKE9iamVjdC5nZXRP d25Qcm9wZXJ0eURlc2NyaXB0b3IoTm9kZUZpbHRlciwgcHJvcGVydGllc1tpXSkpOworfQorPC9z Y3JpcHQ+Cis8c2NyaXB0IHNyYz0iLi4vLi4vcmVzb3VyY2VzL2pzLXRlc3QtcG9zdC5qcyI+PC9z Y3JpcHQ+Cis8L2JvZHk+Cis8L2h0bWw+Cg==