deCOMtaminate WidgetToScreen and ScreenToWidget

RESOLVED FIXED in mozilla1.9.2a1

Status

()

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: reg, Assigned: reg)

Tracking

Trunk
mozilla1.9.2a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
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.
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)
(Assignee)

Comment 2

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

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+
Keywords: checkin-needed
Whiteboard: [needs landing]
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
Last Resolved: 10 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 → ---
(Assignee)

Comment 6

10 years ago
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.
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/eb5029e4fc09, thanks!!!
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 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.