[Qt] Implements a disable appearance for Media Elements of Qt port.
Created attachment 92984 [details] Patch
Comment on attachment 92984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > Source/WebCore/platform/qt/RenderThemeQt.cpp:1228 > + fgColor = Qt::gray; I could have used platformInactiveSelectionForegroundColor or platformInactiveSelectionBackgroundColor but they really look crap on my Linux KDE.
Comment on attachment 92984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > Source/WebCore/platform/qt/RenderThemeQt.cpp:1227 > + if (mediaElement && (!hasSource(mediaElement) || !mediaElement->hasAudio())) Do you really want paint the thumb as disabled if the element has no audio track?
(In reply to comment #3) > (From update of attachment 92984 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > > > Source/WebCore/platform/qt/RenderThemeQt.cpp:1227 > > + if (mediaElement && (!hasSource(mediaElement) || !mediaElement->hasAudio())) > > Do you really want paint the thumb as disabled if the element has no audio track? Should I check instead if maxTimeSeekable > 0 ?
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 92984 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > > > > > Source/WebCore/platform/qt/RenderThemeQt.cpp:1227 > > > + if (mediaElement && (!hasSource(mediaElement) || !mediaElement->hasAudio())) > > > > Do you really want paint the thumb as disabled if the element has no audio track? > > Should I check instead if maxTimeSeekable > 0 ? It depends entirely on what you are trying to accomplish. If you want to make it look like seeking is disabled when a file has video but no audio, the current code is probably fine. How are you hoping to help the user with this UI change?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 92984 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > > > > > > > Source/WebCore/platform/qt/RenderThemeQt.cpp:1227 > > > > + if (mediaElement && (!hasSource(mediaElement) || !mediaElement->hasAudio())) > > > > > > Do you really want paint the thumb as disabled if the element has no audio track? > > > > Should I check instead if maxTimeSeekable > 0 ? > > It depends entirely on what you are trying to accomplish. If you want to make it look like seeking is disabled when a file has video but no audio, the current code is probably fine. > > How are you hoping to help the user with this UI change? What I'm trying to achieve is that when we don't have yet meta-data, no source basically we're not able to play the video, the controls will look disabled. As soon as data comes in then they look active again. If you have Chromium you'll see.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > (From update of attachment 92984 [details] [details] [details] [details]) > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=92984&action=review > > > > > > > > > Source/WebCore/platform/qt/RenderThemeQt.cpp:1227 > > > > > + if (mediaElement && (!hasSource(mediaElement) || !mediaElement->hasAudio())) > > > > > > > > Do you really want paint the thumb as disabled if the element has no audio track? > > > > > > Should I check instead if maxTimeSeekable > 0 ? > > > > It depends entirely on what you are trying to accomplish. If you want to make it look like seeking is disabled when a file has video but no audio, the current code is probably fine. > > > > How are you hoping to help the user with this UI change? > > What I'm trying to achieve is that when we don't have yet meta-data, no source basically we're not able to play the video, the controls will look disabled. As soon as data comes in then they look active again. If you have Chromium you'll see. How about we use the readyState: - HAVE_NOTHING: If preload is none, draw play-button i normal color (so the user can initiate load), everything else disabled. If preload is not none (metadata or auto), draw everything disabled, until.... - HAVE_METADATA: start drawing play-button using normal colors regardless, same goes for other controls, unless you want to delay them to HAVE_CURRENT_DATA? In all cases, if there are errors, we should reflect those by disabling (graying out) all controls.
Created attachment 93166 [details] Patch
Comment on attachment 93166 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93166&action=review > Source/WebCore/platform/qt/RenderThemeQt.cpp:1232 > + if (!mediaElementCanPlay(o)) > + fgColor = Qt::gray; Is disabled always grey with all themes? > Source/WebCore/platform/qt/RenderThemeQt.cpp:1455 > + p.painter->setBrush(getMediaControlForegroundColor(o->parent())); Maybe make it more clear who is the parent here
Created attachment 93202 [details] Patch
(In reply to comment #10) > Created an attachment (id=93202) [details] > Patch I use the palette now. So whatever color is used in Disabled will be picked.
Thanks ;-)
Comment on attachment 93202 [details] Patch Clearing flags on attachment: 93202 Committed r86299: <http://trac.webkit.org/changeset/86299>
All reviewed patches have been landed. Closing bug.
Revision r86299 cherry-picked into qtwebkit-2.2 with commit ba1970d <http://gitorious.org/webkit/qtwebkit/commit/ba1970d>