[WebKit-devel] Zoom actions change proposal (patch)

[Urs Wolfer]

On Sunday 03 May 2009 22:20:28 Paweł Prażak wrote:
> Some comments:
> > 0001-Intendation-cleanup:
> > Note that such a change makres merging with the original file from KHTML
> > harder, but it's okay for me since it improves readabilty.
>
> Hmm, I understand. Is it possible to patch KHTML code to change this (I
> know it's just a detail but I wouldn't want to debug this code ;)

Yeah, that's a nice idea.

> 0001-Zoom-actions-changes:
> > * Are you sue the bool var m_zoomTextOnly is always initiated?
>
> No, I'm not sure, but all of the settings are initialized the same way as I
> see, so I assumed this is the working way. I don't know if this is a good,
> idea but we can move all bit fields to union with an int and init the whole
> union at once. But I'm not sure if it's necessary.

Okay then.

> * "Zoom Normal" is not a very nice text IMHO. What about "Reset zoom"? Or
>
> > what
> > do you suggest?
>
> "Reset Zoom" or "Zoom Reset" are good. I didn't gave much thought about the
> name.
> Maybe "Default Zoom", but reset is probably better. Is there a convention
> established in KDE?

Not one I'm aware of...  I still like "Reset Zoom" best. Probably ask a native 
english speaking person...
>
> * Just thought about it: is it the usual way that the user sets an option
>
> > for
> > one tab, and it gets saved as a global setting? Probably you could add a
> > new
> > option in the HTML options, and respect that option? Though the issue
> > would be
> > that KHTML would not follow this setting... So not really sure how to
> > handle
> > that. Probably your way is the only really working way. Any thoughts,
> > anyone?
>
> FF have global setting as well, so it's not that bad ;) If we make this
> setting per tab how do we save this? per domain? I don't know how other
> people but I only use option to set text only zoom when I'm testing/writing
> the code, so it's not a hot feature anyway. ;)

Okay :) Let it as it is now.
>
> 0002-Code-cleanup:
> > This patch makes merging really harder. I also like nice code styling a
> > lot,
> > but I'm not sure if it is worth for loosing automatic merging...
>
> Yes, now I understand why it looks how it looks. Now I see there is no
> point in style cleanup without cleanup upstream.
>
>  0003-Added-astyle-script-for-kdelibs-style.-Applyed-sctip.patch:
> > Have you manually reviewed all changes? In some rare cases astyle styling
> > is
> > not the optimal thing.
>
> No I didn't, but good to know that astyle isn't perfect. Anyway, I can try
> to rewrite the script to force styling only on files that have no
> imporeted/merged code (but I'm not sure if it makes sense).

The only files which need needs to be merged are in kdewebkit/settings.
Either you or I need to review all astyle changes before committing.

Btw, do you have a KDE SVN account? Otherwise I will commit your changes.

Bye
urs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/webkit-devel/attachments/20090503/80a61f69/attachment.sig