Bug 106624

Summary: offsetWidth/height incorrect for images when zoomed
Product: WebKit Reporter: Emil A Eklund <eae>
Component: Layout and RenderingAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, dglazkov, eric, leviw, noel.gordon, ojan.autocc, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 106741    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Emil A Eklund 2013-01-10 18:19:14 PST
offsetWidth and height are incorrect for images at certain zoom levels due to pixel snapping the value prior to adjusting for zoom.

Downstream chromium bug: http://code.google.com/p/chromium/issues/detail?id=160215
Comment 1 Emil A Eklund 2013-01-10 18:24:34 PST
Created attachment 182236 [details]
Patch
Comment 2 Emil A Eklund 2013-01-10 18:35:18 PST
Comment on attachment 182236 [details]
Patch

Missed a couple of LayoutTests that needs updating.
Comment 3 Emil A Eklund 2013-01-11 15:51:50 PST
Created attachment 182438 [details]
Patch
Comment 4 Levi Weintraub 2013-01-11 17:23:21 PST
Comment on attachment 182438 [details]
Patch

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

> LayoutTests/fast/images/zoomed-offset-size.html:6
> +                zoom: 1.25;

Can we iterate through a range of zoom levels? I'll feel more comfortable with that change :)
Comment 5 Emil A Eklund 2013-01-11 17:29:42 PST
Created attachment 182451 [details]
Patch
Comment 6 Levi Weintraub 2013-01-11 17:30:44 PST
Comment on attachment 182451 [details]
Patch

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

> LayoutTests/fast/images/zoomed-offset-size-expected.txt:1
> +PASS getSize(0.01).imageWidth is 250

That's probably more output than we need, but I like the completeness :) I feel good about this now.
Comment 7 WebKit Review Bot 2013-01-11 18:39:22 PST
Comment on attachment 182451 [details]
Patch

Clearing flags on attachment: 182451

Committed r139537: <http://trac.webkit.org/changeset/139537>
Comment 8 WebKit Review Bot 2013-01-11 18:39:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 noel gordon 2013-01-11 20:23:47 PST
platform/chromium/inspector/styles/device-metrics-fit-window.html went red on chrome mac release with this change, expected?

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&group=%40ToT%20-%20webkit.org&tests=platform%2Fchromium%2Finspector%2Fstyles%2Fdevice-metrics-fit-window.html
Comment 10 Levi Weintraub 2013-01-12 13:13:30 PST
That looks looks like it just needs to be rebaselined. Emil, can you confirm?
Comment 11 Csaba Osztrogonác 2013-01-13 01:41:18 PST
(In reply to comment #7)
> (From update of attachment 182451 [details])
> Clearing flags on attachment: 182451
> 
> Committed r139537: <http://trac.webkit.org/changeset/139537>

It made 2 tests fail on GTK/Qt/EFL - https://bugs.webkit.org/show_bug.cgi?id=106741

Could you check it, please?
Comment 12 Levi Weintraub 2013-01-13 12:47:28 PST
(In reply to comment #11)
> (In reply to comment #7)
> > (From update of attachment 182451 [details] [details])
> > Clearing flags on attachment: 182451
> > 
> > Committed r139537: <http://trac.webkit.org/changeset/139537>
> 
> It made 2 tests fail on GTK/Qt/EFL - https://bugs.webkit.org/show_bug.cgi?id=106741
> 
> Could you check it, please?

Those failures look legitimate. I'm going to roll this out until Emil can take a look.
Comment 13 Levi Weintraub 2013-01-13 12:49:36 PST
(In reply to comment #12)
> 
> Those failures look legitimate. I'm going to roll this out until Emil can take a look.

(I believe this is due to those ports not having sub-pixel layout enabled :( )
Comment 14 Levi Weintraub 2013-01-13 13:50:06 PST
Reverted in http://trac.webkit.org/changeset/139573
Comment 15 Emil A Eklund 2013-01-14 09:37:41 PST
Created attachment 182591 [details]
Patch
Comment 16 Levi Weintraub 2013-01-14 09:43:50 PST
Comment on attachment 182591 [details]
Patch

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

> LayoutTests/fast/images/zoomed-offset-size.html:1
> +<!DOCTYPE html>

You'll want to skip this test on GTK and Qt... sounds like EFL is turning this on today so maybe just let the folks doing that know :)
Comment 17 Emil A Eklund 2013-01-14 09:44:58 PST
Comment on attachment 182591 [details]
Patch

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

>> LayoutTests/fast/images/zoomed-offset-size.html:1
>> +<!DOCTYPE html>
> 
> You'll want to skip this test on GTK and Qt... sounds like EFL is turning this on today so maybe just let the folks doing that know :)

This test _should_ still work for non-subpixel ports. I'd like to leave it on for all ports for now and (quickly) disable it if my assumptions are incorrect if that is alright with you.
Comment 18 Levi Weintraub 2013-01-14 09:45:33 PST
(In reply to comment #17)
> (From update of attachment 182591 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182591&action=review
> 
> >> LayoutTests/fast/images/zoomed-offset-size.html:1
> >> +<!DOCTYPE html>
> > 
> > You'll want to skip this test on GTK and Qt... sounds like EFL is turning this on today so maybe just let the folks doing that know :)
> 
> This test _should_ still work for non-subpixel ports. I'd like to leave it on for all ports for now and (quickly) disable it if my assumptions are incorrect if that is alright with you.

Sounds fine :)
Comment 19 WebKit Review Bot 2013-01-14 10:41:26 PST
Comment on attachment 182591 [details]
Patch

Attachment 182591 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15884212

New failing tests:
fast/frames/removal-before-attach-crash.html
Comment 20 Emil A Eklund 2013-01-14 13:45:34 PST
Comment on attachment 182591 [details]
Patch

Committed r139659: <http://trac.webkit.org/changeset/139659>