Summary: | Don't shrink by default always when zoom in/out is enabled for standalone images | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mario Sanchez Prada <mario> | ||||||||||
Component: | Images | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||
Severity: | Normal | CC: | xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Mario Sanchez Prada
2009-12-20 12:34:18 PST
Created attachment 45291 [details] Patch for proposal 1 (In reply to comment #0) > [...] > 1. Keep the same meaning for ShrinksStandaloneImagesToFit values and just > *add a new setting* to control the initial status of an image when shrinking is > activated: initially shrink it or not. Attached patch would implement this proposal Created attachment 45292 [details] Patch for proposal 2 (In reply to comment #0) > [...] > 2. Change the meaning of ShrinksStandaloneImagesToFit setting so it would > control *only* whether the image is initially shrank or not, having the handler > for the zoom in/out thing always installed. Attached patch would implement this This patch has been overlooked, because it isn't marked for review. Please see <http://webkit.org/coding/contributing.html> for more detail. (In reply to comment #3) > This patch has been overlooked, because it isn't marked for review. Please see <http://webkit.org/coding/contributing.html> for more detail. Sorry, I'll rebuild the patches against the last version of trunk and will add review flags for both of them this time, although obviously, only one of those would get in in the best case :-) Created attachment 58659 [details]
Patch for proposal 1
Here you have a patch for the first proposal done against the latest version of trunk.
Personally, this is the option I like the most, but do not pay much attention to that, just let me know what you think both of the proposal and the implementation :-)
Created attachment 58662 [details]
Patch for proposal 2
...and here is the new patch for the second proposal
This has been stuck for quite a long time and I wonder whether I should do something else other than ping people or keep commenting. Adding Xan Lopez to CC, as he's the maintainer of Epiphany browser, which depends on this bug to get https://bugzilla.gnome.org/show_bug.cgi?id=599997 fixed. Comment on attachment 58659 [details]
Patch for proposal 1
There doesn't seem to be much interested in this patch. R- for lack of tests.
Comment on attachment 58662 [details]
Patch for proposal 2
No tests.
Thanks Adam for reviewing the patches. The lack of tests is a shame, actually, but I wrote those patches almost one year ago when I barely knew what a Layout Test was, so I'm not surprised I didn't add any... :-) Anyway, now I'm busy fixing those bugs related to accessibility in the GTK port so I don't think I'll work for a while on this bug (hope that's ok), unless I find some contiguous spare time at some point to focus on that, but that will be unlikely until december, I'm afraid :-( Once again, thanks a lot for reviewing it. Now I have a clear lead to follow to get this fixed. I think 4 years, together with the fact that this patch does not apply any more, is prudent enough to resolve this as a won't fix. |