Bug 93449

Summary: [EFL] Fixed slider thumb position on vertical slider
Product: WebKit Reporter: KwangYong Choi <ky0.choi>
Component: WebKit EFLAssignee: KwangYong Choi <ky0.choi>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: dglazkov, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Bug Depends on: 93838, 94595    
Bug Blocks:    
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-01
none
Patch
none
Patch
none
Patch none

Description KwangYong Choi 2012-08-08 01:27:26 PDT
The UI and slider coordinates are reversed in case of vertical slider. So, it's required to convert the slider thumb position reversed.

The minimum value of the vertical slider is on the bottom. But,the minimum value of the UI is on the top.
Comment 1 KwangYong Choi 2012-08-08 01:46:56 PDT
Created attachment 157152 [details]
Patch
Comment 2 WebKit Review Bot 2012-08-08 02:40:17 PDT
Comment on attachment 157152 [details]
Patch

Attachment 157152 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13456418

New failing tests:
fast/forms/range/slider-thumb-initial-position-vertical.html
Comment 3 WebKit Review Bot 2012-08-08 02:40:21 PDT
Created attachment 157164 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 4 KwangYong Choi 2012-08-08 04:09:03 PDT
Created attachment 157173 [details]
Patch

Added test result for cr-linux.

I think, all the other ports can pass this test. I'll check and provide expected result for the others.
Comment 5 Gyuyoung Kim 2012-08-08 04:48:25 PDT
Comment on attachment 157173 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157173&action=review

> LayoutTests/ChangeLog:13
> +        * platform/efl/fast/forms/range/slider-thumb-initial-position-vertical-expected.txt: Added.

It looks test cases for chromium port are missing in ChangeLog.

platform/chromium-linux/fast/forms/range/slider-thumb-initial-position-vertical-expected.png
platform/chromium-linux/fast/forms/range/slider-thumb-initial-position-vertical-expected.txt
Comment 6 KwangYong Choi 2012-08-08 05:48:15 PDT
> 
> It looks test cases for chromium port are missing in ChangeLog.
> 
> platform/chromium-linux/fast/forms/range/slider-thumb-initial-position-vertical-expected.png
> platform/chromium-linux/fast/forms/range/slider-thumb-initial-position-vertical-expected.txt

You're right. I will add these to ChangeLog.

I have a question. Do you know why EWS don't show mac port? I try to add expected result for mac port, but it's not checked by EWS. Isn't it necessary to provide?
Comment 7 Gyuyoung Kim 2012-08-08 05:54:21 PDT
(In reply to comment #6)
 
> I have a question. Do you know why EWS don't show mac port? I try to add expected result for mac port, but it's not checked by EWS. Isn't it necessary to provide?

Because mac ews is disappeared when pending patches are over 100. Now, mac ews is disappeared because of many pending patches. 

http://webkit-commit-queue.appspot.com/queue-status/mac-ews

If you need to add a test case to mac port, you have to check if your patch is ok on Mac port as well.
Comment 8 KwangYong Choi 2012-08-08 19:56:37 PDT
Created attachment 157374 [details]
Patch
Comment 9 Gyuyoung Kim 2012-08-08 20:20:41 PDT
Comment on attachment 157374 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157374&action=review

> LayoutTests/ChangeLog:17
> +        * platform/mac/fast/forms/range/slider-thumb-initial-position-vertical-expected.txt: Added.

Will you support gtk, qt ports as well? If not, you need to add these tests to it's Skipped file.
Comment 10 KwangYong Choi 2012-08-10 03:05:29 PDT
Comment on attachment 157374 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157374&action=review

>> LayoutTests/ChangeLog:17
>> +        * platform/mac/fast/forms/range/slider-thumb-initial-position-vertical-expected.txt: Added.
> 
> Will you support gtk, qt ports as well? If not, you need to add these tests to it's Skipped file.

Gyuyoung, I want to make this test as EFL specific. The other ports will work fine. It's about simple slider thumb display thing. But, it's too hard to make expected result on all the other ports.

From my observation, there are many platform specific tests on LayoutTests/platform directory. I think it's possible to make it as EFL platform specific. What do you think?
Comment 11 KwangYong Choi 2012-08-14 20:04:00 PDT
Actually, this patch can be tested with fast/forms/datalist/input-appearance-range-with-datalist.html which has vertical slider with initial value.

But, vertical slider is not displayed properly because the applied theme for vertical slider is same as horizontal slider. So, the height value of the vertical slider is too small. It should be fixed first to pass the above range with datalist test.

I may remove the test in this patch after I fix vertical slider theme problem.
Comment 12 Gyuyoung Kim 2012-08-14 21:07:53 PDT
(In reply to comment #11)
> Actually, this patch can be tested with fast/forms/datalist/input-appearance-range-with-datalist.html which has vertical slider with initial value.
> 
> But, vertical slider is not displayed properly because the applied theme for vertical slider is same as horizontal slider. So, the height value of the vertical slider is too small. It should be fixed first to pass the above range with datalist test.
> I may remove the test in this patch after I fix vertical slider theme problem.

I think you can feel free to modify this patch according to your consideration.

> From my observation, there are many platform specific tests on LayoutTests/platform directory. I think it's possible to make it as EFL platform specific. What do you think?

If there is proper reason to have efl specific test case, I think EFL port can have the specific test case as mac, gtk and qt ports
Comment 13 KwangYong Choi 2012-08-23 03:55:47 PDT
Created attachment 160125 [details]
Patch

This patch is can be tested by fast/forms/datalist/fast/forms/datalist/input-appearance-range-with-datalist.html now.
Comment 14 KwangYong Choi 2012-08-28 18:56:08 PDT

*** This bug has been marked as a duplicate of bug 94595 ***
Comment 15 Gyuyoung Kim 2012-08-29 00:38:48 PDT
Comment on attachment 160125 [details]
Patch

Clearing r?