When working on another case (https://bugs.webkit.org/show_bug.cgi?id=108852) I noticed that the colorpicker is no longer saving values. If you click on the color swatch and open up the colorpicker, it will let you change the color in the popup, but the value never gets saved. This is caused by the following line: Source/WebCore/inspector/front-end/StylesSidebarPane.js - var scrollerElement = hasSpectrum ? self._parentPane._computedStylePane.element.parentElement : null; This fixes it: + var scrollerElement = hasSpectrum ? self._parentPane._computedStylePane.bodyElement : null; I'm working on a patch for this.
Created attachment 187572 [details] Patch to fix colorpicker save issue Sorry if this is not in the correct format, trying to follow the guide for creating a patch and may not have done it right.
@Vlad: I believe the issue is related to your recent changes to the sidebar structure. Could you see if anything else is broken in the StylesSidebarPane and MetricsSidebarPane (by plain looking at the code that may use the changed DOM elements)? The solution suggested by Brian does not fix the broken behavior when the spectrum popup should follow the swatch when the sidebar pane stack is scrolled. Could you advise on the right solution?
Comment on attachment 187572 [details] Patch to fix colorpicker save issue Does the spectrum follow the swatch when the sidebar is scrolled?
You are right, the popup does not follow the swatch while the sidebar is scrolled with the proposed patch. (In reply to comment #3) > (From update of attachment 187572 [details]) > Does the spectrum follow the swatch when the sidebar is scrolled?
Not saying that we *should* do this, but this code does cause everything to work: var scrollerElement = hasSpectrum ? self._parentPane._parentView._parentView.element.parentNode : null; The scroll event seems to now be happening on the div[class=split-view-contents split-view-contents-vertical split-view-sidebar-right]. Surely there is a better way to get that element from the stylessidebarpane than the way above, though. (In reply to comment #3) > (From update of attachment 187572 [details]) > Does the spectrum follow the swatch when the sidebar is scrolled?
(In reply to comment #5) > Not saying that we *should* do this, but this code does cause everything to work: > > var scrollerElement = hasSpectrum ? self._parentPane._parentView._parentView.element.parentNode : null; > > The scroll event seems to now be happening on the div[class=split-view-contents split-view-contents-vertical split-view-sidebar-right]. Surely there is a better way to get that element from the stylessidebarpane than the way above, though. Correct. That's why I was asking Vlad for advice. The new structure must have better accessors than ugly constructs like this one :)
Created attachment 188050 [details] Patch
Comment on attachment 188050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188050&action=review > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1786 > + var scrollerElement = self._parentPane.container().scrollTarget(); Can the scrollerElement change while the color picker is active? (E.g. if the user resizes the web inspector window, which can alter the sidebar pane representation) > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1788 > + console.log("detached scroll listener from ", scrollerElement); Please remove debug logging > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1822 > + console.log("attached scroll listener to ", scrollerElement); Ditto
Created attachment 188052 [details] Patch
Created attachment 188059 [details] Patch
Comment on attachment 188059 [details] Patch r=me
Comment on attachment 188059 [details] Patch Rejecting attachment 188059 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 188059, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: rce/WebKit/chromium/webkit/media/crypto/ppapi/cdm --revision 173055 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 56>At revision 173055. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/16514150
Created attachment 188071 [details] Patch
Comment on attachment 188071 [details] Patch Clearing flags on attachment: 188071 Committed r142745: <http://trac.webkit.org/changeset/142745>
All reviewed patches have been landed. Closing bug.