Bug 47084 - [EFL] Support viewport configuration and add new arguments for WebKit EFL
Summary: [EFL] Support viewport configuration and add new arguments for WebKit EFL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 46772 47395
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-04 06:45 PDT by Gyuyoung Kim
Modified: 2010-10-13 05:15 PDT (History)
7 users (show)

See Also:


Attachments
Prototype Patch (13.15 KB, patch)
2010-10-07 03:48 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (13.80 KB, patch)
2010-10-12 06:01 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2010-10-04 06:45:13 PDT
targetDensityDpi argument was added to Viewport Arguments. So, EFL port need to support that as well.
Comment 1 Gyuyoung Kim 2010-10-05 18:07:14 PDT
To adjust viewport configuration to efl port, I think current structure of viewport handling needs to be changed. I am considering how to change it.
Comment 2 Gyuyoung Kim 2010-10-05 18:18:40 PDT
(In reply to comment #1)
> I think current structure of viewport handling needs to be changed.

I mean viewport structure needs to be changed in Efl port. :)
Comment 3 Gyuyoung Kim 2010-10-07 03:48:47 PDT
Created attachment 70064 [details]
Prototype Patch

I make a prototype patch according to the changed viewport implementation in WebCore. I have considered how to set desktop_width, device_width, device_height and available_size. But, I don't find appropriate solution yet.

For now, I use windowRect() for the device_width/height and available_size.
And, I don't know how to use dpi value. So, I just let ewk_view store the dpi value.

I'd like to get any advices related to viewport implementation. I continue to think this patch.
Comment 4 Joone Hur 2010-10-07 08:30:38 PDT
(In reply to comment #3)
> Created an attachment (id=70064) [details]
> Prototype Patch
> 
> I make a prototype patch according to the changed viewport implementation in WebCore. I have considered how to set desktop_width, device_width, device_height and available_size. But, I don't find appropriate solution yet.

The deskopWidth means the width of the viewport that works well for most webpages designed for a desktop browser. Usually, we use 980px for this like Safari on the iPhone.

The device_width/device_height is the size of the screen, but these values should be changed when the screen is rotated.

The available size means the area of the visible viewport. So in case of the iPhone, the width of visible viewport is the device width.

> 
> For now, I use windowRect() for the device_width/height and available_size.
> And, I don't know how to use dpi value. So, I just let ewk_view store the dpi value.

I think chrome()->pageRect() would be better for getting the available_size.
In case of getting the device_width/height, you can use the screen size for them.
In addition, WebCore::findConfigurationForViewportData has the DPI parameter for computing the devicePixelRatio of the ViewportAttributes.
Comment 5 Kenneth Rohde Christiansen 2010-10-07 09:50:11 PDT
Please keep in mind that most ports now call it ViewportAttributes and not ViewportConfiguration.
Comment 6 Gyuyoung Kim 2010-10-12 06:01:27 PDT
Created attachment 70526 [details]
Patch

I modify previous patch. 

I use pageRect() to set availableSize, on the other hand, windowRect() is used by deviceWidth.

    149 +    int available_width = (int) priv->page->chrome()->client()->pageRect().width();
    150 +    int available_height = (int) priv->page->chrome()->client()->pageRect().height();
    151 +
    152 +    int device_width = (int) priv->page->chrome()->client()->windowRect().width();
    153 +    int device_height = (int) priv->page->chrome()->client()->windowRect().height();

But, width of pageRect() is 20 pixel less than windowRect()'s because EWebLauncher has a pink rectangle inside window.
The computeViewportAttributes() returns the biggest width value between parameters. Eventually, deviceWidth is returned by the computeViewportAttributes(). In mobile sites, EWebLauncher doesn't fit the page perfectly because deviceWidth is 20 pixel bigger than pageRect(). However, I think this is right for other device. As you know, other device doesn't has internal rectangle like EWebLauncher.


BTW, I wonder why default dpi value is 160.

    result.devicePixelRatio = float(deviceDPI / 160.0);

I made a dpi method in Bug 47537. However, the method returns 87 on my linux PC. As you know, 87 is less than 160. Thus, deviceWidth is multiplied by the ratio. Eventually, Layout is broken.

    // Resolve non-'auto' width and height to pixel values.
    if (deviceDPI != 1.0) {
        deviceWidth /= result.devicePixelRatio;
        deviceHeight /= result.devicePixelRatio;

        if (args.width != ViewportArguments::ValueAuto)
            args.width /= result.devicePixelRatio;
        if (args.height != ViewportArguments::ValueAuto)
            args.height /= result.devicePixelRatio;
    }

For the time being, I hardcode the dpi value to 160.
Comment 7 Kenneth Rohde Christiansen 2010-10-12 07:11:05 PDT
> BTW, I wonder why default dpi value is 160.
> 
>     result.devicePixelRatio = float(deviceDPI / 160.0);
> 
> I made a dpi method in Bug 47537. However, the method returns 87 on my linux PC. As you know, 87 is less than 160. Thus, deviceWidth is multiplied by the ratio. Eventually, Layout is broken.
> 
>     // Resolve non-'auto' width and height to pixel values.
>     if (deviceDPI != 1.0) {
>         deviceWidth /= result.devicePixelRatio;
>         deviceHeight /= result.devicePixelRatio;
> 
>         if (args.width != ViewportArguments::ValueAuto)
>             args.width /= result.devicePixelRatio;
>         if (args.height != ViewportArguments::ValueAuto)
>             args.height /= result.devicePixelRatio;
>     }
> 
> For the time being, I hardcode the dpi value to 160.

The device dpi handling needs work in WebCore - it is not working yet. It is 160 as that is what the iPhone (the device introducing the viewport meta tag) uses. Thus, using 160 doesnt break pages.
Comment 8 WebKit Commit Bot 2010-10-13 05:15:06 PDT
Comment on attachment 70526 [details]
Patch

Clearing flags on attachment: 70526

Committed r69650: <http://trac.webkit.org/changeset/69650>
Comment 9 WebKit Commit Bot 2010-10-13 05:15:13 PDT
All reviewed patches have been landed.  Closing bug.