Bug 130308 - bad values from HTMLImageElement x and y attributes (CSSOM-View)
Summary: bad values from HTMLImageElement x and y attributes (CSSOM-View)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-16 12:53 PDT by Alan Stearns
Modified: 2014-04-05 06:52 PDT (History)
8 users (show)

See Also:


Attachments
uses testharness.js (1.80 KB, text/html)
2014-03-16 12:53 PDT, Alan Stearns
no flags Details
Patch (5.75 KB, patch)
2014-04-02 06:26 PDT, Jeongeun Kim
no flags Details | Formatted Diff | Diff
Patch (5.35 KB, patch)
2014-04-03 05:20 PDT, Jeongeun Kim
no flags Details | Formatted Diff | Diff
Patch for landing (5.52 KB, patch)
2014-04-05 06:14 PDT, Jeongeun Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Stearns 2014-03-16 12:53:10 PDT
Created attachment 226854 [details]
uses testharness.js

The x and y attributes on an HTMLImageElement are supposed to give the left/top border edge positions of the image's layout box. In WebKit/Blink, they always return 0. I've submitted this testharness.js testcase to the CSSWG test repository. It succeeds in Firefox and Opera 12.
Comment 1 Jeongeun Kim 2014-04-02 05:35:45 PDT
Alan, are you looking into this?
If not, I'd like to handle it.
Comment 2 Mihnea Ovidenie 2014-04-02 05:40:50 PDT
(In reply to comment #1)
> Alan, are you looking into this?
> If not, I'd like to handle it.

Please go ahead, Alan is not working on this issue.
Comment 3 Jeongeun Kim 2014-04-02 06:26:00 PDT
Created attachment 228391 [details]
Patch
Comment 4 Jeongeun Kim 2014-04-02 06:30:12 PDT
I uploaded patch for this issue. 
I'm not sure whether I can use the attached test case created by Alan Stearns or not.
If reviewers and Alan give me feedback, I'll update it.
Thanks,
Comment 5 Darin Adler 2014-04-02 09:54:31 PDT
Comment on attachment 228391 [details]
Patch

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

The right basic idea here, but incorrect implementation.

> Source/WebCore/html/HTMLImageElement.cpp:382
> +    document().updateStyleIfNeeded();

The correct call to make here is:

    document().updateLayoutIgnorePendingStylesheets();

See Element::offsetLeft for an example of how to do it.

> Source/WebCore/html/HTMLImageElement.cpp:389
> +    if (renderer->needsLayout())
> +        document().updateLayout();

This is not needed. Please don’t add it.

> Source/WebCore/html/HTMLImageElement.cpp:397
> +    document().updateStyleIfNeeded();

Same comment as above.

> Source/WebCore/html/HTMLImageElement.cpp:404
> +    if (renderer->needsLayout())
> +        document().updateLayout();

This is not needed. Please don’t add it.
Comment 6 Jeongeun Kim 2014-04-03 05:20:11 PDT
Created attachment 228496 [details]
Patch
Comment 7 Jeongeun Kim 2014-04-03 05:23:56 PDT
Darin, Thank you for review.
I updated my patch. Please look into that.
Thanks,
Comment 8 Gyuyoung Kim 2014-04-05 05:27:09 PDT
Comment on attachment 228496 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +        https://bugs.webkit.org/show_bug.cgi?id=130308

Wrong place for bug title and url.

bad values from HTMLImageElement x and y attributes (CSSOM-View)
https://bugs.webkit.org/show_bug.cgi?id=130308

> LayoutTests/ChangeLog:3
> +        According to CSSOM-View, âinterface HTMLImageElementâ,

ditto.
Comment 9 Jeongeun Kim 2014-04-05 06:14:45 PDT
Created attachment 228672 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2014-04-05 06:52:37 PDT
Comment on attachment 228672 [details]
Patch for landing

Clearing flags on attachment: 228672

Committed r166833: <http://trac.webkit.org/changeset/166833>
Comment 11 WebKit Commit Bot 2014-04-05 06:52:42 PDT
All reviewed patches have been landed.  Closing bug.