[webkit-reviews] A patch I have!

[John Sullivan]

In general we are less likely to accept patches for adding UI  
features than we are to accept patches for website compatibility  
issues, because there are more design tradeoffs involved with UI  
features and they need to be vetted across a wider set of people.

In this particular case, it's arguable whether the lack of support  
for keyboard zooming in PDFs is a missing feature or a bug. We've had  
a number of requests for the PDF view in Safari to support the  
existing Make Text Bigger and Make Text Smaller commands for zooming.  
This is not a perfect match, because zooming in PDFs changes the size  
of images as well as text, whereas these commands in HTML only affect  
text size, not image size. Even so, I think it's a close enough match  
that it's a clear improvement to make these commands affect PDF  
pages. These commands are built into WebKit so that any client could  
choose to implement UI for them; they have the keyboard equivalents  
of command-plus and command-minus in Safari.

So I do think the general idea of your patch is something we want to  
do in WebKit. However, the implementation in your patch isn't the  
best way to do it, because it's disconnected from the existing WebKit  
feature. The approach I'd like to see here is to make PDFView  
implement the _web_WebDocumentTextSizing protocol that's defined in  
WebDocumentInternal.h. That way, the existing menu and toolbar items  
for text sizing in Safari (and other WebKit clients that have  
implemented UI for this feature) will work for PDFs also.

There are two implementations of this protocol in WebKit currently  
that you could look at as models for how to do it in PDFView. One is  
in WebHTMLView, the other is in WebTextView. I haven't looked at the  
PDF code myself enough yet to know how straightforward it will be to  
map WebKit's "text size multiplier" concept to PDF's "zoom level"  
concept, but that mapping would be the heart of the work to do this  
feature.

I'm pretty likely to do this work at some point if nobody beats me to  
it, but feel free to beat me to it!

John

On Jun 7, 2005, at 11:49 PM, Devin Lane wrote:

> Reviewers:
>
> I'm not sure if you are accepting patches for non-bugs, but I made  
> a patch that adds the ability for the plus and minus keys to  
> control the zoom level of a PDF that is being viewed in Safari.
>
> Here is the patch file:
>
> <zoompatch.txt>
>
> Since this is my first patch, I'm not at all expecting it to be  
> included in the actual source, but I thought I would pass it along  
> just to see if you were interested.
>
> I did not preform any regression tests, since this patch affects  
> neither the renderer or the javascript engine.
>
> Let me know if you have any questions,
>
> Devin Lane
> Cocoa Programmer
> Editor-in-Chief of Tech Pep (Benson H.S. Newspaper)
> Phone: (503) 775-4906
> E-Mail: devin at devserver.org
>
> _______________________________________________
> webkit-reviews mailing list
> webkit-reviews at opendarwin.org
> http://www.opendarwin.org/mailman/listinfo/webkit-reviews
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.macosforge.org/pipermail/webkit-reviews/attachments/20050608/e5da76f8/attachment.html