[Webkit-unassigned] [Bug 43395] [GTK/EFL] rendering was broken in naver.com

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43395


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64495|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #14 from Antonio Gomes <tonikitoo at webkit.org>  2010-08-17 11:03:05 PST ---
(From update of attachment 64495)
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 65425)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,19 @@
> +2010-08-16  Ryuan Choi  <ryuan.choi at gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [GTK/EFL] rendering was broken in naver.com
> +        https://bugs.webkit.org/show_bug.cgi?id=43395
> +
> +        Implement RenderThemeEfl::systemFont to render "Missing plugin" when we
> +        don't have proper plugin.
> +
> +        * platform/efl/RenderThemeEfl.cpp:
> +        (WebCore::RenderThemeEfl::setDefaultFontSize):
> +        (WebCore::RenderThemeEfl::defaultGUIFont):
> +        (WebCore::RenderThemeEfl::systemFont):
> +        * platform/efl/RenderThemeEfl.h:

Could you please explain why RenderThemeEfl::systemFont fixes it? Maybe mentioning the code path.

Also you are not fixing the Gtk bug, but only EFL. Please make it explicit in the patch/ChangeLog.

> --- WebCore/platform/efl/RenderThemeEfl.cpp	(revision 65424)
> +++ WebCore/platform/efl/RenderThemeEfl.cpp	(working copy)
> @@ -26,6 +26,7 @@
>  #include "config.h"
>  #include "RenderThemeEfl.h"
>  
> +#include "CSSValueKeywords.h"
>  #include "FileSystem.h"
>  #include "Frame.h"
>  #include "FrameView.h"
> @@ -631,6 +632,8 @@ void RenderThemeEfl::themeChanged()
>      applyPartDescriptions();
>  }
>  
> +float RenderThemeEfl::defaultFontSize = 16.0f;
> +
>  RenderThemeEfl::RenderThemeEfl(Page* page)
>      : RenderTheme()
>      , m_page(page)
> @@ -985,10 +988,28 @@ bool RenderThemeEfl::paintSearchField(Re
>      return paintThemePart(o, SearchField, i, rect);
>  }
>  
> -void RenderThemeEfl::systemFont(int, FontDescription&) const
> +void RenderThemeEfl::setDefaultFontSize(int size)
> +{
> +    defaultFontSize = size;
> +}
> +
> +const String& RenderThemeEfl::defaultGUIFont()

I personally do not like defaultGUIFont() naming much.

>  {
> -    // If you remove this notImplemented(), replace it with an comment that explains why.

Please respect the request above :-)

> -    notImplemented();
> +    DEFINE_STATIC_LOCAL(String, fontFace, ("Sans"));
> +    return fontFace;
> +}
> +
> +void RenderThemeEfl::systemFont(int propId, FontDescription& fontDescription) const
> +{
> +    // copied from RenderThemeChromiumSkia
> +    float fontSize = defaultFontSize;
> +
> +    fontDescription.firstFamily().setFamily(defaultGUIFont());
> +    fontDescription.setSpecifiedSize(fontSize);
> +    fontDescription.setIsAbsoluteSize(true);
> +    fontDescription.setGenericFamily(FontDescription::NoFamily);
> +    fontDescription.setWeight(FontWeightNormal);
> +    fontDescription.setItalic(false);
>  }
>  
>  }
> Index: WebCore/platform/efl/RenderThemeEfl.h
> ===================================================================
> --- WebCore/platform/efl/RenderThemeEfl.h	(revision 65424)
> +++ WebCore/platform/efl/RenderThemeEfl.h	(working copy)
> @@ -142,6 +142,14 @@ public:
>  
>      virtual void adjustSliderThumbStyle(CSSStyleSelector*, RenderStyle*, Element*) const;
>      virtual bool paintSliderThumb(RenderObject*, const PaintInfo&, const IntRect&);
> +
> +    static void setDefaultFontSize(int size);
> +
> +protected:
> +    static const String& defaultGUIFont();
> +
> +    static float defaultFontSize;
> +
>  private:
>      void createCanvas();
>      void createEdje();
> Index: WebKit/efl/ChangeLog
> ===================================================================
> --- WebKit/efl/ChangeLog	(revision 65425)
> +++ WebKit/efl/ChangeLog	(working copy)
> @@ -1,3 +1,17 @@
> +2010-08-16  Ryuan Choi  <ryuan.choi at gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [GTK/EFL] rendering was broken in naver.com
> +        https://bugs.webkit.org/show_bug.cgi?id=43395
> +
> +        call RenderThemeEfl::setDefaultFontSize when initialzed and user want to
> +        change.

Start the phrase with capital, and correct the spelling of "initialzed".

>  #include <Ecore.h>
> @@ -552,6 +553,7 @@ static Ewk_View_Private_Data* _ewk_view_
>      priv->page_settings->setLoadsImagesAutomatically(true);
>      priv->page_settings->setDefaultFixedFontSize(12);
>      priv->page_settings->setDefaultFontSize(16);
> +    WebCore::RenderThemeEfl::setDefaultFontSize(16);

I wonder the difference between page_settings->setDefaultFontSize(16) and RenderThemeEfl::setDefaultFontSize(16);

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.