Closed Bug 476726 Opened 14 years ago Closed 14 years ago

deCOMtaminate WidgetToScreen and ScreenToWidget

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: reg, Assigned: reg)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v1.0 (obsolete) — Splinter Review
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.
Attachment #360378 - Flags: superreview?(roc)
Attachment #360378 - Flags: review?(roc)
Attachment #360378 - Flags: superreview?(roc)
Attachment #360378 - Flags: superreview+
Attachment #360378 - Flags: review?(roc)
Attachment #360378 - Flags: review+
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)
Attached patch v1.1 [Backout: Comment 4] (obsolete) — Splinter Review
Fixed and updated for Qt changes.
Attachment #360378 - Attachment is obsolete: true
Attachment #362146 - Flags: superreview?(roc)
Attachment #362146 - Flags: review?(roc)
Attachment #362146 - Flags: superreview?(roc)
Attachment #362146 - Flags: superreview+
Attachment #362146 - Flags: review?(roc)
Attachment #362146 - Flags: review+
Comment on attachment 362146 [details] [diff] [review]
v1.1
[Backout: Comment 4]


http://hg.mozilla.org/mozilla-central/rev/719dca3419b5
Attachment #362146 - Attachment description: v1.1 → v1.1 [Checkin: Comment 3]
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.2a1
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'
}
Attachment #362146 - Attachment description: v1.1 [Checkin: Comment 3] → v1.1 [Backout: Comment 4]
Attachment #362146 - Attachment is obsolete: true
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/eb5029e4fc09, thanks!!!
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
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 !?
This patch broke building on Qt. I filed bug 480689 for the breakage.
You need to log in before you can comment on or make changes to this bug.