Bug 145639 - [GTK] Volume bar is broken
Summary: [GTK] Volume bar is broken
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: ChangSeok Oh
URL: http://www.w3.org/2010/05/video/media...
Keywords:
: 132253 142491 (view as bug list)
Depends on:
Blocks: 149032 149396
  Show dependency treegraph
 
Reported: 2015-06-04 01:46 PDT by Xabier Rodríguez Calvar
Modified: 2017-03-11 10:58 PST (History)
10 users (show)

See Also:


Attachments
Wrong (41.95 KB, image/png)
2015-06-04 01:46 PDT, Xabier Rodríguez Calvar
no flags Details
Right (9.07 KB, image/png)
2015-06-04 01:47 PDT, Xabier Rodríguez Calvar
no flags Details
Patch (2.07 KB, patch)
2015-09-09 03:40 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (4.11 KB, patch)
2015-09-09 22:24 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xabier Rodríguez Calvar 2015-06-04 01:46:30 PDT
Created attachment 254259 [details]
Wrong

Currently it looks like the attached screenshot and it is completely non-funcional.

I suspect something might be removed from the RenderTheme* that removed the only part that were handled in C++, like the volume path and the way it was filed up to the volume level. If it were like that, funny that nobody asked me...
Comment 1 Xabier Rodríguez Calvar 2015-06-04 01:47:32 PDT
Created attachment 254260 [details]
Right

You see here the volume path inside the volume bar, it is filled up to its volume level and the thumb is smaller.
Comment 2 Michael Catanzaro 2015-07-30 13:50:13 PDT
Works in 2.8, so this must have broken between February and June.
Comment 3 ChangSeok Oh 2015-09-09 03:28:10 PDT
I'm investigating this. Let me take over it.
Comment 4 ChangSeok Oh 2015-09-09 03:40:42 PDT
Created attachment 260855 [details]
Patch
Comment 5 Xabier Rodríguez Calvar 2015-09-09 05:28:58 PDT
(In reply to comment #4)
> Created attachment 260855 [details]
> Patch

Good! This does the trick.

Have you run the tests, including pixel and maybe remove any possible failing expectations?

Carlos, this patch has to enter the next release, otherwise volume bar can be crippled.
Comment 6 ChangSeok Oh 2015-09-09 06:11:49 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Created attachment 260855 [details]
> > Patch
> 
> Good! This does the trick.
> 
> Have you run the tests, including pixel and maybe remove any possible
> failing expectations?
I have not. Let me see if there is something that I can do.
BTW did we block some tests due to this issue before?
Comment 7 Xabier Rodríguez Calvar 2015-09-09 06:55:56 PDT
(In reply to comment #6)
> BTW did we block some tests due to this issue before?

I haven't checked either, but there were some tests (with pixel) checking the volume bar so I bet that there must be something in the expectations or we just didn't care and it was failing so far.
Comment 8 ChangSeok Oh 2015-09-09 22:24:23 PDT
Created attachment 260909 [details]
Patch
Comment 9 ChangSeok Oh 2015-09-09 22:33:29 PDT
media/click-volume-bar-not-pausing.html
media/video-volume-slider.html
media/volume-bar-empty-when-muted.html

These seem relevant.
media/click-volume-bar-not-pausing.html and media/volume-bar-empty-when-muted.html are unblocked here. But for media/video-volume-slider.html, it looks more touch required. It is flacky. It passes manually, but not with TestRunner.
I think a timing issue involved here. So I would like to handle it in another ticket by modifying the test.
Comment 10 ChangSeok Oh 2015-09-09 22:39:51 PDT
*** Bug 142491 has been marked as a duplicate of this bug. ***
Comment 11 ChangSeok Oh 2015-09-09 22:40:21 PDT
*** Bug 132253 has been marked as a duplicate of this bug. ***
Comment 12 ChangSeok Oh 2015-09-09 23:51:13 PDT
https://bugs.webkit.org/show_bug.cgi?id=149032
Comment 13 Xabier Rodríguez Calvar 2015-09-10 01:04:05 PDT
LGTM
Comment 14 Philippe Normand 2015-09-10 01:19:44 PDT
Comment on attachment 260909 [details]
Patch

Thanks for fixing this ChangSeok!
Comment 15 ChangSeok Oh 2015-09-10 01:23:44 PDT
Comment on attachment 260909 [details]
Patch

@philn My pleasure =)
Comment 16 WebKit Commit Bot 2015-09-10 02:10:22 PDT
Comment on attachment 260909 [details]
Patch

Clearing flags on attachment: 260909

Committed r189566: <http://trac.webkit.org/changeset/189566>
Comment 17 WebKit Commit Bot 2015-09-10 02:10:28 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Michael Catanzaro 2016-05-15 17:20:39 PDT
Reopening because test media/volume-bar-empty-when-muted.html has been flaky since this patch was committed. Previously the test always failed; now it almost always fails, but sometimes passes.
Comment 19 Michael Catanzaro 2016-05-15 17:41:52 PDT
Updated expectations in r200936.
Comment 20 ChangSeok Oh 2016-05-19 01:35:55 PDT
> Reopening because test media/volume-bar-empty-when-muted.html has been flaky
> since this patch was committed. Previously the test always failed; now it
> almost always fails, but sometimes passes.

Thanks for the report, Michael. Unfortunately, I have no available linux machine for now so I would not be able to investigate this (as well as other bugs assigned to me) until having a new machine. (probably until the next month) If anyone is interested in this bug, please feel free to take it over from me. If not, I will look into this once again.
Comment 21 Michael Catanzaro 2016-05-19 06:36:18 PDT
> so I would not be able to investigate this (as well as other
> bugs assigned to me) until having a new machine.

Oh that's fine, I wasn't really expecting you to... just keeping track of failing tests.

FWIW, to overload this bug further: I notice the volume bar is draggable, that's not good.

(In reply to comment #20)
> Unfortunately, I have no available linux
> machine for now 

Oh no. Oh no oh no oh no oh no oh no. I hope you survive the month. :(
Comment 22 Michael Catanzaro 2016-05-19 06:53:06 PDT
(In reply to comment #21) 
> FWIW, to overload this bug further: I notice the volume bar is draggable,
> that's not good.

Nevermind, it doesn't seem to be a problem anymore. I noticed that in the ancient WebKit shipped by Debian stable, you can drag the entire volume bar (not just the slider) around the entire web view unless you carefully click exactly on the slider.