Bug 70478 - [EFL] Unregister 'viewport,changed' signal in EWebLauncher
Summary: [EFL] Unregister 'viewport,changed' signal in EWebLauncher
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-19 23:43 PDT by Gyuyoung Kim
Modified: 2011-10-26 01:12 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.88 KB, patch)
2011-10-19 23:47 PDT, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (5.25 KB, patch)
2011-10-20 21:09 PDT, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Blogspot with viewport support (37.87 KB, image/png)
2011-10-20 21:11 PDT, Gyuyoung Kim 		no flags Details
Blogspot without viewport support (76.23 KB, image/png)
2011-10-20 21:11 PDT, Gyuyoung Kim 		no flags Details
m.wikipedia.org with viewport support (104.98 KB, image/png)
2011-10-20 21:12 PDT, Gyuyoung Kim 		no flags Details
m.wikipedia.org without viewport support (142.75 KB, image/png)
2011-10-20 21:12 PDT, Gyuyoung Kim 		no flags Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2011-10-19 23:43:54 PDT 
'viewport,changed' signal was registered by EWebLauncher in order to test viewport meta tag. However, it sets wrong layout to some web sites which have viewport meta tag. Because, host PC doesn't support proper DPI value though viewport is using DPI value to compute layout values. This signal is for mobile device.
Comment 1 Gyuyoung Kim 2011-10-19 23:47:25 PDT 
Created attachment 111730 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-10-20 05:47:41 PDT 
Comment on attachment 111730 [details]
Patch

Can you explain in more details what the problem with the current code is? I couldn't understand it from the ChangeLog message.

Plus, if you're really going to unregister the signal, you can also remove the whole on_viewport_changed function as well.
Comment 3 Gyuyoung Kim 2011-10-20 21:09:15 PDT 
Created attachment 111893 [details]
Patch
Comment 4 Gyuyoung Kim 2011-10-20 21:11:05 PDT 
Created attachment 111894 [details]
Blogspot with viewport support
Comment 5 Gyuyoung Kim 2011-10-20 21:11:39 PDT 
Created attachment 111895 [details]
Blogspot without viewport support
Comment 6 Gyuyoung Kim 2011-10-20 21:12:17 PDT 
Created attachment 111896 [details]
m.wikipedia.org with viewport support
Comment 7 Gyuyoung Kim 2011-10-20 21:12:48 PDT 
Created attachment 111897 [details]
m.wikipedia.org without viewport support
Comment 8 Gyuyoung Kim 2011-10-20 21:24:43 PDT 
(In reply to comment #2)
> (From update of attachment 111730 [details])
> Can you explain in more details what the problem with the current code is? I couldn't understand it from the ChangeLog message.

I attach screenshots to let you understand this patch's goal. In blogspot case, blogspot has viewport meta tag as below,
<meta content='width=1100' name='viewport'/>

Then, EWebLauncher sets layout according to viewport computation logic with device dpi. However, viewport algorithm computes the DPI ratio based on 160(iphone 3Gs dpi). viewport callback of EWebLauncher sets zoom-in via the DPI ratio. So, EWebLauncher can't set proper layout because host PC usually has low DPI. 

Especially, it is difficult to test mobile sites on EWebLauncher because of this problem. For example, en.m.wikipedia.org

> Plus, if you're really going to unregister the signal, you can also remove the whole on_viewport_changed function as well.

Fixed.
Comment 9 Raphael Kubo da Costa (:rakuco) 2011-10-22 08:42:21 PDT 
Alright, informal r+.

Sorry for the delay, I wasn't in the bug's CC list.
Comment 10 Eric Seidel (no email) 2011-10-26 00:52:04 PDT 
Comment on attachment 111893 [details]
Patch

rs=me.
Comment 11 WebKit Review Bot 2011-10-26 01:12:10 PDT 
Comment on attachment 111893 [details]
Patch

Clearing flags on attachment: 111893

Committed r98444: <http://trac.webkit.org/changeset/98444>
Comment 12 WebKit Review Bot 2011-10-26 01:12:15 PDT 
All reviewed patches have been landed.  Closing bug.