Closed
Bug 476726
Opened 14 years ago
Closed 14 years ago
deCOMtaminate WidgetToScreen and ScreenToWidget
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: reg, Assigned: reg)
References
()
Details
Attachments
(1 file, 2 obsolete files)
32.91 KB,
patch
|
Details | Diff | 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)
Assignee | ||
Comment 2•14 years ago
|
||
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 3•14 years ago
|
||
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]
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.2a1
Comment 4•14 years ago
|
||
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
Updated•14 years ago
|
Comment 5•14 years ago
|
||
(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 :-/
Assignee | ||
Comment 6•14 years ago
|
||
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•14 years ago
|
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/eb5029e4fc09, thanks!!!
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Comment 8•14 years ago
|
||
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•14 years ago
|
||
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.
Description
•