104498 – [EFL][WK2] Enable TEXT_AUTOSIZING

Bug 104498 - [EFL][WK2] Enable TEXT_AUTOSIZING

Summary: [EFL][WK2] Enable TEXT_AUTOSIZING

Status:	RESOLVED WONTFIX

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebKit EFL (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Nobody

URL:
Keywords:

Depends on:
Blocks:	99074
	Show dependency tree / graph

Reported:	2012-12-09 15:54 PST by Jaehun Lim
Modified:	2017-03-11 10:39 PST (History)
CC List:	10 users (show)

See Also:

Attachments
Patch (1.21 KB, patch) 2013-08-06 23:49 PDT, Divakar	no flags	Details \| Formatted Diff \| Diff
Patch (2.62 KB, text/plain) 2013-08-08 02:38 PDT, Divakar	eflews.bot: commit-queue-	Details
Patch (3.77 KB, text/plain) 2013-08-08 05:58 PDT, Divakar	no flags	Details
Patch (3.77 KB, text/plain) 2013-08-08 06:00 PDT, Divakar	no flags	Details
Patch (3.77 KB, patch) 2013-08-08 06:03 PDT, Divakar	no flags	Details \| Formatted Diff \| Diff
Patch (4.01 KB, patch) 2013-08-12 04:48 PDT, Divakar	gyuyoung.kim: review- eflews.bot: commit-queue-	Details \| Formatted Diff \| Diff
Show Obsolete (5) 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 Jaehun Lim 2012-12-09 15:54:59 PST

Enable TEXT_AUTOSIZING feature.

Comment 1 Divakar 2013-08-06 23:49:33 PDT

Created attachment 208239 [details]
Patch

Comment 2 Divakar 2013-08-07 01:53:22 PDT

Added patch for enabling TEXT_AUTOSIZING feature on EFL port.

Comment 3 Jaehun Lim 2013-08-07 02:06:20 PDT

(In reply to comment #2)
> Added patch for enabling TEXT_AUTOSIZING feature on EFL port.

You have to edit Tools/Scripts/webkitperl/FeatureList.pm. Please see http://trac.webkit.org/wiki/AddingFeatures.

And you have to resolve the HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP warning issue, too. http://trac.webkit.org/browser/trunk/Source/WebCore/page/Settings.cpp#L131.

Comment 4 Divakar 2013-08-08 02:38:59 PDT

Created attachment 208320 [details]
Patch

Comment 5 Divakar 2013-08-08 02:39:48 PDT

Updated the patch.
I did not find any warning related to HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP

Comment 6 EFL EWS Bot 2013-08-08 02:59:09 PDT

Comment on attachment 208320 [details]
Patch

Attachment 208320 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1432111

Comment 7 EFL EWS Bot 2013-08-08 03:18:06 PDT

Comment on attachment 208320 [details]
Patch

Attachment 208320 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1396120

Comment 8 Divakar 2013-08-08 05:58:55 PDT

Created attachment 208333 [details]
Patch

Comment 9 Divakar 2013-08-08 06:00:15 PDT

Created attachment 208334 [details]
Patch

Comment 10 Divakar 2013-08-08 06:03:16 PDT

Created attachment 208335 [details]
Patch

Comment 11 Divakar 2013-08-08 06:05:06 PDT

Corrected error and updated the patch.

Comment 12 Benjamin Poulain 2013-08-08 13:21:45 PDT

Comment on attachment 208335 [details]
Patch

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

> Source/WTF/wtf/Platform.h:1061
> +#if PLATFORM(EFL)
> +#define HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP 0
> +#endif
> +

This is not okay.

Comment 13 Divakar 2013-08-11 21:55:36 PDT

(In reply to comment #12)
> (From update of attachment 208335 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208335&action=review
> 
> > Source/WTF/wtf/Platform.h:1061
> > +#if PLATFORM(EFL)
> > +#define HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP 0
> > +#endif
> > +
> 
> This is not okay.
do you want it to be enabled?

Comment 14 Benjamin Poulain 2013-08-12 00:04:31 PDT

(In reply to comment #13) 
> > This is not okay.
> do you want it to be enabled?

No!!!
This should not be in this file.

A quick grep shows it is not defined anywhere, you should probably remove HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP from settings.

Comment 15 Divakar 2013-08-12 04:48:49 PDT

Created attachment 208534 [details]
Patch

Comment 16 EFL EWS Bot 2013-08-12 05:31:57 PDT

Comment on attachment 208534 [details]
Patch

Attachment 208534 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1441567

Comment 17 Chris Dumez 2013-08-12 10:22:28 PDT

Comment on attachment 208534 [details]
Patch

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

> Source/WebCore/page/Settings.cpp:131
>      , m_textAutosizingWindowSizeOverride(320, 480)

Why are we keeping this line if we are removing HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP?

Comment 18 Jaehun Lim 2013-08-12 17:34:56 PDT

(In reply to comment #17)
> (From update of attachment 208534 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208534&action=review
> 
> > Source/WebCore/page/Settings.cpp:131
> >      , m_textAutosizingWindowSizeOverride(320, 480)
> 
> Why are we keeping this line if we are removing HACK_FORCE_TEXT_AUTOSIZING_ON_DESKTOP?

"windows size" is used to adjust the font size. If it is not override, "visible contents size" is used instead of it. http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/TextAutosizer.cpp#L116

I think this line is necessary to activate TextAutosizing for the big screen. But I don't like it.

Johe Mellor filed the related bug, https://bugs.webkit.org/show_bug.cgi?id=94371

Comment 19 Gyuyoung Kim 2013-12-25 18:44:36 PST

Comment on attachment 208534 [details]
Patch

r- because of failing efl-wk2.

Comment 20 Michael Catanzaro 2017-03-11 10:39:14 PST

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.