Bug 148531 – Document window.NodeFilter properties

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 260068 [details]
Patch

[Geoffrey Garen]

Comment on attachment 260068 [details]
Patch

r=me

[Chris Dumez]

Committed r189059: <http://trac.webkit.org/changeset/189059>

[Brent Fulgham]

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.

[Chris Dumez]

(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
    };

[Chris Dumez]

(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,