custom CSS cursors ignore hotspot values embedded in CUR files
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2012 02:28:00 +0000 (02:28 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2012 02:28:00 +0000 (02:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100059

Patch by Rick Byers <rbyers@chromium.org> on 2012-11-15
Reviewed by Kenneth Russell.

Source/WebCore:

Add reading the hotspot values to the ICOImageDecoder (for CUR files only),
and plumb it through so that the existing calls to ImageSource::getHotSpot
actually return the hot spot value when there is one.

Tests: fast/events/mouse-cursor.html, fast/events/mouse-cursor-multiframecur.html

* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::getHotSpot):
* platform/graphics/chromium/DeferredImageDecoder.cpp:
(WebCore::DeferredImageDecoder::hotSpot):
(WebCore::DeferredImageDecoder::hotSpotAtIndex):
* platform/graphics/chromium/DeferredImageDecoder.h:
(DeferredImageDecoder):
* platform/image-decoders/ImageDecoder.h:
(WebCore::ImageDecoder::hotSpot):
(WebCore::ImageDecoder::hotSpotAtIndex):
(ImageDecoder):
* platform/image-decoders/ico/ICOImageDecoder.cpp:
(WebCore::ICOImageDecoder::hotSpot):
(WebCore::ICOImageDecoder::hotSpotAtIndex):
(WebCore::ICOImageDecoder::processDirectory):
(WebCore::ICOImageDecoder::readDirectoryEntry):
* platform/image-decoders/ico/ICOImageDecoder.h:
(ICOImageDecoder):
(IconDirectoryEntry):

LayoutTests:

Add mouse cursor test cases that use CUR files that have embedded hotspot values.

* fast/events/mouse-cursor-expected.txt:
* fast/events/mouse-cursor-multiframecur-expected.txt: Added.
* fast/events/mouse-cursor-multiframecur.html: Copied from LayoutTests/fast/events/mouse-cursor.html.
* fast/events/mouse-cursor.html:
* fast/events/resources/greenbox-3frames.cur: Added.
* fast/events/resources/greenbox-hotspot35-4.cur: Added.
* fast/events/resources/greenbox-hotspot5-4.cur: Added.
* platform/mac/TestExpectations: Skip the multi-frame case on mac because it causes a hang

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@134874 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/mouse-cursor-expected.txt
LayoutTests/fast/events/mouse-cursor-multiframecur-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/mouse-cursor-multiframecur.html [new file with mode: 0644]
LayoutTests/fast/events/mouse-cursor.html
LayoutTests/fast/events/resources/greenbox-3frames.cur [new file with mode: 0644]
LayoutTests/fast/events/resources/greenbox-hotspot35-4.cur [new file with mode: 0644]
LayoutTests/fast/events/resources/greenbox-hotspot5-4.cur [new file with mode: 0644]
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp
Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.h
Source/WebCore/platform/image-decoders/ImageDecoder.h
Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp
Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.h

index b349de0b02d2af6e128f972941b7b05b2d16617e..af61cf6016dedd69fe2832fd4bf00ec8f200c5b6 100644 (file)
@@ -1,3 +1,21 @@
+2012-11-15  Rick Byers  <rbyers@chromium.org>
+
+        custom CSS cursors ignore hotspot values embedded in CUR files
+        https://bugs.webkit.org/show_bug.cgi?id=100059
+
+        Reviewed by Kenneth Russell.
+
+        Add mouse cursor test cases that use CUR files that have embedded hotspot values.
+
+        * fast/events/mouse-cursor-expected.txt:
+        * fast/events/mouse-cursor-multiframecur-expected.txt: Added.
+        * fast/events/mouse-cursor-multiframecur.html: Copied from LayoutTests/fast/events/mouse-cursor.html.
+        * fast/events/mouse-cursor.html:
+        * fast/events/resources/greenbox-3frames.cur: Added.
+        * fast/events/resources/greenbox-hotspot35-4.cur: Added.
+        * fast/events/resources/greenbox-hotspot5-4.cur: Added.
+        * platform/mac/TestExpectations: Skip the multi-frame case on mac because it causes a hang
+
 2012-11-15  Kenichi Ishibashi  <bashi@chromium.org>
 
         Use complex path for the reference text in fast/text/international/combining-marks-position.html
index b558e95ee4069657fa33ec6ef9d0444ad671373e..7e0f2a61cfd8b40f49fe544b8e3446a416dd5fe0 100644 (file)
@@ -45,6 +45,15 @@ Cursor Info: type=Custom hotSpot=0,0 image=25x25
 TEST CASE: Image with explicit hot spot outside image at (30,30)
 Cursor Info: type=Custom hotSpot=0,0 image=25x25
 
+TEST CASE: Image with implicit hot spot at (5,4)
+Cursor Info: type=Custom hotSpot=5,4 image=25x25
+
+TEST CASE: Image with explicit hot spot at (20,10) overriding implicit hot spot
+Cursor Info: type=Custom hotSpot=20,10 image=25x25
+
+TEST CASE: Image with implicit hot spot outside image at (35,4)
+Cursor Info: type=Custom hotSpot=0,0 image=25x25
+
 TEST CASE: Over large image with fallback to pointer
 Cursor Info: type=Hand hotSpot=0,0
 
diff --git a/LayoutTests/fast/events/mouse-cursor-multiframecur-expected.txt b/LayoutTests/fast/events/mouse-cursor-multiframecur-expected.txt
new file mode 100644 (file)
index 0000000..bb05803
--- /dev/null
@@ -0,0 +1,18 @@
+Test that mouse cursors are applied correctly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Bug 100059
+
+
+TEST CASE: Implicit default cursor
+Cursor Info: type=IBeam hotSpot=0,0
+
+TEST CASE: CUR file with 3 frames, largest of which (2nd frame) is 20x12 with hotspot at (18,11).
+Cursor Info: type=Custom hotSpot=18,11 image=20x12
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/mouse-cursor-multiframecur.html b/LayoutTests/fast/events/mouse-cursor-multiframecur.html
new file mode 100644 (file)
index 0000000..fc6e6f4
--- /dev/null
@@ -0,0 +1,93 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../js/resources/js-test-pre.js"></script>
+<style type="text/css">
+</style>
+</head>
+<body>
+<p id="description"></p>
+<p><a href=https://bugs.webkit.org/show_bug.cgi?id=100550>Bug 100059</a></p>
+<div id="test-container">
+  <div>Implicit default cursor</div>
+  <div style='cursor: url(resources/greenbox-3frames.cur), pointer'>CUR file with 3 frames, largest of which (2nd frame) is 20x12 with hotspot at (18,11).</div>
+</div>
+<br/>
+<div id="console"></div>
+<script>
+var imagesLeftToLoad = 0;
+
+function onImageLoad(e) {
+
+    // This debug output is non-deterministic and contains absolute URLs - only show when
+    // not running in DRT
+    if (!window.testRunner)
+        debug( 'Event "' + e.type + '": ' + e.target.src);
+        
+    imagesLeftToLoad--;
+    if (imagesLeftToLoad == 0)
+        runTests();
+    if (imagesLeftToLoad < 0)
+        testFailed('Got more load/error callback than expected.');
+}
+
+function onImageUnexpected(e) {
+       imagesLeftToLoad--;
+    testFailed('Got unexpected \'' + e.type + '\' event for image: ' + e.target.src); 
+}
+
+function runTests() {
+    // Can't do anything useful here without eventSender
+    if (window.eventSender) {
+        var nodesToTest = document.querySelectorAll('#test-container > div');
+        for (var i = 0; i < nodesToTest.length; i++) {
+            var node = nodesToTest[i];
+            debug('TEST CASE: ' + node.textContent);
+
+            // Make sure the node is visible and move the mouse over top of it.
+            document.body.scrollTop = node.offsetTop - 50;
+            eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop - document.body.scrollTop + 3);
+
+            // Get details of the current mouse cursor.
+            // Note that we could return structured data which we then validate, but that's a lot more
+            // work and is redundant with relying on the expected output anyway.  Better to just dump
+            // it and inspect that it matches the description.
+            debug('Cursor Info: ' + window.internals.getCurrentCursorInfo(document));
+            debug('');
+        }
+        // This text is redundant with the test output - hide it
+        document.getElementById('test-container').style.display = 'none';
+    }
+    
+    finishJSTest();
+}
+
+description("Test that mouse cursors are applied correctly.");
+
+if (!window.eventSender) {
+    testFailed('This test requires DumpRenderTree');
+}
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+    window.jsTestIsAsync = true;
+}
+
+// Now wait for each image to load or fail to load before starting tests.
+// Without this we can get null images in the cursors - eg. no known size.
+var imagesToLoad = [
+    { url: 'resources/greenbox-3frames.cur' } ];
+imagesLeftToLoad = imagesToLoad.length;
+for (var i = 0; i < imagesToLoad.length; i++) {
+    var img = new Image();
+    var expectError = imagesToLoad[i].error; 
+    img.addEventListener('load', expectError ? onImageUnexpected : onImageLoad);
+    img.addEventListener('error', expectError ? onImageLoad : onImageUnexpected);
+    img.src = imagesToLoad[i].url;
+}
+
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index f821d93b43b7f33464a178ed1df53b93f3f83b79..634d2d6d5394d5a6887c3664439c195b3db25dfc 100644 (file)
@@ -28,6 +28,9 @@
   <div style='cursor: url(resources/greenbox.png) 20 10, pointer'>Image with explicit hot spot at (20,10)</div>
   <div style='cursor: url(resources/greenbox.png) -1 -1, pointer'>Image with explicit hot spot at (-1,-1)</div>
   <div style='cursor: url(resources/greenbox.png) 30 30, pointer'>Image with explicit hot spot outside image at (30,30)</div>
+  <div style='cursor: url(resources/greenbox-hotspot5-4.cur), pointer'>Image with implicit hot spot at (5,4)</div>
+  <div style='cursor: url(resources/greenbox-hotspot5-4.cur) 20 10, pointer'>Image with explicit hot spot at (20,10) overriding implicit hot spot</div>
+  <div style='cursor: url(resources/greenbox-hotspot35-4.cur), pointer'>Image with implicit hot spot outside image at (35,4)</div>
   <div style='cursor: url(resources/onload-image.png), pointer'>Over large image with fallback to pointer</div>
   <div style='cursor: url(#greenbox), pointer'>SVG cursor</div>
   <div style='cursor: url(mouse-cursor.html), url(unknown-scheme:cursor.png), pointer'>Multiple invalid cursors with fallback to pointer</div>
@@ -67,7 +70,12 @@ function runTests() {
         for (var i = 0; i < nodesToTest.length; i++) {
             var node = nodesToTest[i];
             debug('TEST CASE: ' + node.textContent);
-            eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop + 3);
+
+            // Make sure the node is visible and move the mouse over top of it.
+            document.body.scrollTop = node.offsetTop - 50;
+            eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop - document.body.scrollTop + 3);
+
+            // Get details of the current mouse cursor.
             // Note that we could return structured data which we then validate, but that's a lot more
             // work and is redundant with relying on the expected output anyway.  Better to just dump
             // it and inspect that it matches the description.
@@ -98,6 +106,8 @@ if (window.testRunner) {
 var imagesToLoad = [
     { url: 'resources/greenbox.png' },
     { url: 'resources/onload-image.png' },
+    { url: 'resources/greenbox-hotspot5-4.cur' },
+    { url: 'resources/greenbox-hotspot35-4.cur' },
     { url: 'doesntexist.png', error: true },
     { url: 'unknown-scheme:cursor.png', error: true },
     { url: 'mouse-cursor.html', error: true },
diff --git a/LayoutTests/fast/events/resources/greenbox-3frames.cur b/LayoutTests/fast/events/resources/greenbox-3frames.cur
new file mode 100644 (file)
index 0000000..2b09d64
Binary files /dev/null and b/LayoutTests/fast/events/resources/greenbox-3frames.cur differ
diff --git a/LayoutTests/fast/events/resources/greenbox-hotspot35-4.cur b/LayoutTests/fast/events/resources/greenbox-hotspot35-4.cur
new file mode 100644 (file)
index 0000000..92226c5
Binary files /dev/null and b/LayoutTests/fast/events/resources/greenbox-hotspot35-4.cur differ
diff --git a/LayoutTests/fast/events/resources/greenbox-hotspot5-4.cur b/LayoutTests/fast/events/resources/greenbox-hotspot5-4.cur
new file mode 100644 (file)
index 0000000..5ae7c9c
Binary files /dev/null and b/LayoutTests/fast/events/resources/greenbox-hotspot5-4.cur differ
index d2d1b8507b0f4834d287fa479a04a7de9482d2bc..7e61d53d661dc1b8616cf5287cd438ae6d980ae2 100644 (file)
@@ -1185,3 +1185,6 @@ webkit.org/b/100142 css3/filters/effect-reference.html [ Fail ]
 
 # Mountain Lion and prior do not support custom media data loading
 Bug(jernoble) [ MountainLion Lion SnowLeopard ] media/video-src-blob.html
+
+# Hangs safari apparently due to an issue with weird multi-frame CUR image files
+webkit.org/b/101811 fast/events/mouse-cursor-multiframecur.html [ Skip ]
index 559d1b1af7ef28487f2aa76b1cfa10c4f821af4d..0ae53c8d70cbe1275ddcd10c505897c97aa3f648 100644 (file)
@@ -1,3 +1,36 @@
+2012-11-15  Rick Byers  <rbyers@chromium.org>
+
+        custom CSS cursors ignore hotspot values embedded in CUR files
+        https://bugs.webkit.org/show_bug.cgi?id=100059
+
+        Reviewed by Kenneth Russell.
+
+        Add reading the hotspot values to the ICOImageDecoder (for CUR files only),
+        and plumb it through so that the existing calls to ImageSource::getHotSpot
+        actually return the hot spot value when there is one.
+
+        Tests: fast/events/mouse-cursor.html, fast/events/mouse-cursor-multiframecur.html
+
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::getHotSpot):
+        * platform/graphics/chromium/DeferredImageDecoder.cpp:
+        (WebCore::DeferredImageDecoder::hotSpot):
+        (WebCore::DeferredImageDecoder::hotSpotAtIndex):
+        * platform/graphics/chromium/DeferredImageDecoder.h:
+        (DeferredImageDecoder):
+        * platform/image-decoders/ImageDecoder.h:
+        (WebCore::ImageDecoder::hotSpot):
+        (WebCore::ImageDecoder::hotSpotAtIndex):
+        (ImageDecoder):
+        * platform/image-decoders/ico/ICOImageDecoder.cpp:
+        (WebCore::ICOImageDecoder::hotSpot):
+        (WebCore::ICOImageDecoder::hotSpotAtIndex):
+        (WebCore::ICOImageDecoder::processDirectory):
+        (WebCore::ICOImageDecoder::readDirectoryEntry):
+        * platform/image-decoders/ico/ICOImageDecoder.h:
+        (ICOImageDecoder):
+        (IconDirectoryEntry):
+
 2012-11-15  Kenichi Ishibashi  <bashi@chromium.org>
 
         Make OpenTypeVerticalData be ref-counted
index 56857a43f548423e8bd92d322d96d810c2bb679a..0f46d703c7a8c6dbdbd2d89650211c343ef0b0cd 100644 (file)
@@ -121,9 +121,9 @@ IntSize ImageSource::frameSizeAtIndex(size_t index, RespectImageOrientationEnum
     return size;
 }
 
-bool ImageSource::getHotSpot(IntPoint&) const
+bool ImageSource::getHotSpot(IntPoint& hotSpot) const
 {
-    return false;
+    return m_decoder ? m_decoder->hotSpot(hotSpot) : false;
 }
 
 size_t ImageSource::bytesDecodedToDetermineProperties() const
index ccbb3357a75e0f5bb2cdae174f2c4bf0e63e9985..33d949604deebd6d566f38a8b011418002b06fbd 100644 (file)
@@ -219,4 +219,9 @@ SkBitmap DeferredImageDecoder::createLazyDecodingBitmap()
     return bitmap;
 }
 
+bool DeferredImageDecoder::hotSpot(IntPoint& hotSpot) const
+{
+    return m_actualDecoder ? m_actualDecoder->hotSpot(hotSpot) : false;
+}
+
 } // namespace WebCore
index 33709d9b5989921108663e3afc0f715a4d5e315e..b795049f0c83ad886022434836cd7722ef0cf54f 100644 (file)
@@ -62,6 +62,7 @@ public:
     bool frameHasAlphaAtIndex(size_t index) const;
     unsigned frameBytesAtIndex(size_t index) const;
     ImageOrientation orientation() const;
+    bool hotSpot(IntPoint&) const;
 
 private:
     explicit DeferredImageDecoder(ImageDecoder* actualDecoder);
index 8c0db3929f54208b47fa36382736bfb315647f57..2b5c9836a39d213e89b639b668dea125eb69ca98 100644 (file)
@@ -376,6 +376,10 @@ namespace WebCore {
         void setMaxNumPixels(int m) { m_maxNumPixels = m; }
 #endif
 
+        // If the image has a cursor hot-spot, stores it in the argument
+        // and returns true. Otherwise returns false.
+        virtual bool hotSpot(IntPoint&) const { return false; }
+
         virtual void reportMemoryUsage(MemoryObjectInfo*) const;
 
     protected:
index 2db3ecf6ce7fcc732ce2435dde4dc9798c686308..a3e2952648a198888e66e47d1fc192e68261da1f 100644 (file)
@@ -133,6 +133,24 @@ bool ICOImageDecoder::setFailed()
     return ImageDecoder::setFailed();
 }
 
+bool ICOImageDecoder::hotSpot(IntPoint& hotSpot) const
+{
+    // When unspecified, the default frame is always frame 0. This is consistent with
+    // BitmapImage where currentFrame() starts at 0 and only increases when animation is
+    // requested.
+    return hotSpotAtIndex(0, hotSpot);
+}
+
+bool ICOImageDecoder::hotSpotAtIndex(size_t index, IntPoint& hotSpot) const
+{
+    if (index >= m_dirEntries.size() || m_fileType != CURSOR)
+        return false;
+
+    hotSpot = m_dirEntries[index].m_hotSpot;
+    return true;
+}
+
+
 // static
 bool ICOImageDecoder::compareEntries(const IconDirectoryEntry& a, const IconDirectoryEntry& b)
 {
@@ -232,13 +250,11 @@ bool ICOImageDecoder::processDirectory()
 
     // See if this is an icon filetype we understand, and make sure we have at
     // least one entry in the directory.
-    enum {
-        ICON = 1,
-        CURSOR = 2,
-    };
     if (((fileType != ICON) && (fileType != CURSOR)) || (!idCount))
         return setFailed();
 
+    m_fileType = static_cast<FileType>(fileType);
+
     // Enlarge member vectors to hold all the entries.
     m_dirEntries.resize(idCount);
     m_bmpReaders.resize(idCount);
@@ -287,7 +303,13 @@ ICOImageDecoder::IconDirectoryEntry ICOImageDecoder::readDirectoryEntry()
         height = 256;
     IconDirectoryEntry entry;
     entry.m_size = IntSize(width, height);
-    entry.m_bitCount = readUint16(6);
+    if (m_fileType == CURSOR) {
+        entry.m_bitCount = 0;
+        entry.m_hotSpot = IntPoint(readUint16(4), readUint16(6));
+    } else {
+        entry.m_bitCount = readUint16(6);
+        entry.m_hotSpot = IntPoint();
+    }
     entry.m_imageOffset = readUint32(12);
 
     // Some icons don't have a bit depth, only a color count.  Convert the
index 31d91c81902fb0d2c69dc0a3618cbdc54bf6b713..a909deb80835e82d58a88a8956c3378724cd9f2f 100644 (file)
@@ -56,6 +56,7 @@ namespace WebCore {
         // avoid accessing deleted memory, especially when calling this from
         // inside BMPImageReader!
         virtual bool setFailed();
+        virtual bool hotSpot(IntPoint&) const;
 
     private:
         enum ImageType {
@@ -64,9 +65,15 @@ namespace WebCore {
             PNG,
         };
 
+        enum FileType {
+            ICON = 1,
+            CURSOR = 2,
+        };
+
         struct IconDirectoryEntry {
             IntSize m_size;
             uint16_t m_bitCount;
+            IntPoint m_hotSpot;
             uint32_t m_imageOffset;
         };
 
@@ -109,6 +116,10 @@ namespace WebCore {
         // could be decoded.
         bool processDirectoryEntries();
 
+        // Stores the hot-spot for |index| in |hotSpot| and returns true,
+        // or returns false if there is none.
+        bool hotSpotAtIndex(size_t index, IntPoint& hotSpot) const;
+
         // Reads and returns a directory entry from the current offset into
         // |data|.
         IconDirectoryEntry readDirectoryEntry();
@@ -122,6 +133,9 @@ namespace WebCore {
         // BMPImageReader takes over this will not be updated further.
         size_t m_decodedOffset;
 
+        // Which type of file (ICO/CUR) this is.
+        FileType m_fileType;
+
         // The headers for the ICO.
         typedef Vector<IconDirectoryEntry> IconDirectoryEntries;
         IconDirectoryEntries m_dirEntries;