Bug 142784 - [WinCairo] Video position is incorrect when located inside a frame.
Summary: [WinCairo] Video position is incorrect when located inside a frame.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-17 09:04 PDT by peavo
Modified: 2015-03-18 09:18 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.22 KB, patch)
2015-03-17 09:08 PDT, peavo
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2015-03-17 09:04:51 PDT
We need to take the enclosing frame's position into account, when finding the video position.
Comment 1 peavo 2015-03-17 09:08:29 PDT
Created attachment 248850 [details]
Patch
Comment 2 Brent Fulgham 2015-03-17 09:17:05 PDT
Comment on attachment 248850 [details]
Patch

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

This looks good to me. Could you please use "positionInWIndow" instead of "posInWindow" when you land it?

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:207
> +    IntPoint posInWindow(m_lastPaintRect.location());

I think this should be called "positionInWIndow".
Comment 3 peavo 2015-03-17 09:22:10 PDT
(In reply to comment #2)
> Comment on attachment 248850 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248850&action=review
> 
> This looks good to me. Could you please use "positionInWIndow" instead of
> "posInWindow" when you land it?
> 
> > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:207
> > +    IntPoint posInWindow(m_lastPaintRect.location());
> 
> I think this should be called "positionInWIndow".

Thanks :) Will do.
Comment 4 Brent Fulgham 2015-03-17 09:25:46 PDT
Comment on attachment 248850 [details]
Patch

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

>>> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:207
>>> +    IntPoint posInWindow(m_lastPaintRect.location());
>> 
>> I think this should be called "positionInWIndow".
> 
> Thanks :) Will do.

Of course, I meant "positionInWindow", not "positionInWIndow" ;-)
Comment 5 peavo 2015-03-17 09:36:15 PDT
(In reply to comment #4)
> Comment on attachment 248850 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248850&action=review
> 
> >>> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:207
> >>> +    IntPoint posInWindow(m_lastPaintRect.location());
> >> 
> >> I think this should be called "positionInWIndow".
> > 
> > Thanks :) Will do.
> 
> Of course, I meant "positionInWindow", not "positionInWIndow" ;-)

Exactly ;)
Comment 6 peavo 2015-03-17 14:35:22 PDT
Committed r181665: <http://trac.webkit.org/changeset/181665>
Comment 7 Csaba Osztrogonác 2015-03-18 01:55:14 PDT
(In reply to comment #6)
> Committed r181665: <http://trac.webkit.org/changeset/181665>

This WinCairo fix broke the WinCairo build, it is so funny. :))

..\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(215): error C2664: 'WebCore::IntPoint WebCore::Widget::convertToContainingWindow(const WebCore::IntPoint &) const' : cannot convert argument 1 from 'WebCore::FloatPoint' to 'const WebCore::IntRect &' [C:\cygwin\home\webkitbot\win-cairo-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
          Reason: cannot convert from 'WebCore::FloatPoint' to 'const WebCore::IntRect'
          No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
c:\cygwin\home\webkitbot\win-cairo-release\build\source\webcore\html\htmlelement.cpp(492): warning C4701: potentially uninitialized local variable 'c' used [C:\cygwin\home\webkitbot\win-cairo-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
Comment 8 peavo 2015-03-18 09:18:40 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > Committed r181665: <http://trac.webkit.org/changeset/181665>
> 
> This WinCairo fix broke the WinCairo build, it is so funny. :))
> 
> ..\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(215): error
> C2664: 'WebCore::IntPoint WebCore::Widget::convertToContainingWindow(const
> WebCore::IntPoint &) const' : cannot convert argument 1 from
> 'WebCore::FloatPoint' to 'const WebCore::IntRect &'
> [C:\cygwin\home\webkitbot\win-cairo-release\build\Source\WebCore\WebCore.
> vcxproj\WebCore.vcxproj]
>           Reason: cannot convert from 'WebCore::FloatPoint' to 'const
> WebCore::IntRect'
>           No user-defined-conversion operator available that can perform
> this conversion, or the operator cannot be called
> c:\cygwin\home\webkitbot\win-cairo-
> release\build\source\webcore\html\htmlelement.cpp(492): warning C4701:
> potentially uninitialized local variable 'c' used
> [C:\cygwin\home\webkitbot\win-cairo-release\build\Source\WebCore\WebCore.
> vcxproj\WebCore.vcxproj]

Sorry for this, committed fix in <http://trac.webkit.org/changeset/181694>