Bug 93449 - [EFL] Fixed slider thumb position on vertical slider
Summary: [EFL] Fixed slider thumb position on vertical slider
Status: RESOLVED DUPLICATE of bug 94595
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: KwangYong Choi
URL:
Keywords:
Depends on: 93838 94595
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-08 01:27 PDT by KwangYong Choi
Modified: 2012-08-29 00:38 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.41 KB, patch)
2012-08-08 01:46 PDT, KwangYong Choi
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-01 (314.36 KB, application/zip)
2012-08-08 02:40 PDT, WebKit Review Bot
no flags Details
Patch (9.85 KB, patch)
2012-08-08 04:09 PDT, KwangYong Choi
no flags Details | Formatted Diff | Diff
Patch (15.80 KB, patch)
2012-08-08 19:56 PDT, KwangYong Choi
no flags Details | Formatted Diff | Diff
Patch (10.86 KB, patch)
2012-08-23 03:55 PDT, KwangYong Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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?