476726 - deCOMtaminate WidgetToScreen and ScreenToWidget

Status: RESOLVED FIXED

Description

[Jeremy Lea]

Created attachment 360378 [details] [diff] [review]
v1.0

This is a small patch to fix replace WidgetToScreen and ScreenToWidget with WidgetToScreenOffset, which just returns an nsIntPoint.  Passes tests on Linux and Windows, and seems OK on Mac, although the ObjectiveC code could do with a special look.  OS/2 and Qt changes as a courtesy - totally untested.  Should have no performance downside.

I noticed there is a larger bug for nsIWidget.h, which I've added as a blocker.  I'm not sure if anyone is working on that, but i thought I would file a separate bug for this.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 360378 [details] [diff] [review]
v1.0

+    NS_IMETHOD_(nsIntPoint) WidgetToScreenOffset() = 0;

I would just make this
  virtual nsIntPoint WidgetToScreenOffset() = 0;

+    aRect.MoveTo(WidgetToScreenOffset());
+    aRect.SizeTo(mBounds.Size());

  aRect = nsRect(WidgetToScreenOffset(), mBounds.Size());
(two occurrences)

Comment 2

[Jeremy Lea]

Created attachment 362146 [details] [diff] [review]
v1.1
[Backout: Comment 4]

Fixed and updated for Qt changes.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 362146 [details] [diff] [review]
v1.1
[Backout: Comment 4]


http://hg.mozilla.org/mozilla-central/rev/719dca3419b5

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 362146 [details] [diff] [review]
v1.1
[Backout: Comment 4]


Backed out:
http://hg.mozilla.org/mozilla-central/rev/a3f2ff43ea11
http://hg.mozilla.org/mozilla-central/rev/405c4bcbd3b3

See for example:
{
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234716420.1234716905.27890.gz
WINNT 5.2 mozilla-central unit test on 2009/02/15 08:47:00

nsTextStore.cpp
e:/builds/moz2_slave/mozilla-central-win32-unittest/build/widget/src/windows/nsTextStore.cpp(645) : error C2039: 'WidgetToScreen' : is not a member of 'nsWindow'
        e:\builds\moz2_slave\mozilla-central-win32-unittest\build\widget\src\windows\nsWindow.h(122) : see declaration of 'nsWindow'
e:/builds/moz2_slave/mozilla-central-win32-unittest/build/widget/src/windows/nsTextStore.cpp(689) : error C2039: 'WidgetToScreen' : is not a member of 'nsWindow'
        e:\builds\moz2_slave\mozilla-central-win32-unittest\build\widget\src\windows\nsWindow.h(122) : see declaration of 'nsWindow'
}

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> http://hg.mozilla.org/mozilla-central/rev/a3f2ff43ea11
> http://hg.mozilla.org/mozilla-central/rev/405c4bcbd3b3

Arf, copied the wrong bug title :-/

Comment 6

[Jeremy Lea]

Created attachment 363008 [details] [diff] [review]
Fix build problems

Why does it always seem someone bitrots my patches just when I'm done testing them... ;-)

Revised patch, with fixes for build failures.  Changes are minor and obvious, so no re-review.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/eb5029e4fc09, thanks!!!

Comment 8

[Serge Gautherie (:sgautherie)]

2 probably missed comments:
{
/view/src/nsView.cpp
    * line 407 -- // to compute their true screen position. This could mean that WidgetToScreen

/widget/src/gtk2/nsWindow.cpp
    * line 3201 -- // in scroll_event_cb(), so use ScreenToWidget to translate the screen root coordinates into
}

Also, what about Beos and Photon ports: need to file follow-up bugs !?

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

This patch broke building on Qt. I filed bug 480689 for the breakage.