Hi Vladislav,
I have several comments for the fix:
1.
if(preferredLocation != null) {
location = toFind;
if (!leftToRight) {
location.x -= size.width;
}
} else {
location.x = screenLocation.x + mouseEvent.getX();
location.y = screenLocation.y + mouseEvent.getY() + 20;
In case "preferredLocation != null" creation of new object
Point location = new Point();
is not used, so you could put location = new Point() in else block
2. adjustInsets changes values of arguments. So return the same value
and assigning
workingBounds = adjustInsets(workingBounds, insets);
looks unnecessary
3. I run the test via jtreg and it failed on my environment with the
attached log. Could you please investigate that?
Regards, Pavel
Hello Pavel, all,
thank you for the review, please find new webrev here:
http://cr.openjdk.java.net/~vkarnauk/7123767/webrev.03/
Please see my comments inline.
Regards,
- Vlad
On 8/31/2012 8:50 PM, Pavel Porvatov wrote:
Hi Vladislav,
I have several comments about the fix:
1. getDrawingRect: now localInsets should be requested only if
localRect.contains(toFind) is true
2.
+ // Something's really wrong here - use the screen
that our
+ // Component belongs to
+ if (workingBounds == null)
+ workingBounds =
insideComponent.getGraphicsConfiguration().getBounds();
a. You should always use {...} for if construction
Fixed.
b. It seems we should invoke adjustInsets there. To reduce the code I
think you can rename getDrawingRect into getGraphicsConfiguration and
return GraphicsConfiguration there. And only when
GraphicsConfiguration is found calculate adjusted bounds...
Agreed. Fixed.
3. getDrawingRect comment should have two ")" at the end
Fixed.
Abou the test:
1. You should use Toolkit.realSync after setVisible
Fixed.
2. I run the test on jdk1.8.0b51 and it passes. Looks strange...
The reason why it didn't fail on non-patched JDK8 is because line 184
183 Component component = e.getChild();
184 if (component.isShowing()) {
185 if
(!config.equals(getGCByPoint(component.getLocationOnScreen()))) {
returned "false" as AWT event queue was processed slower than test
thread, thus my check never actually happened. I've changed these
logic (please see new webrev), now this condition is processed
correctly. Test works on patched JDK and fails on non-patched JDK6, 7,
and 8.
Regards, Pavel
Hi Pavel, all,
please find new webrev here:
http://cr.openjdk.java.net/~vkarnauk/7123767/webrev.02/
Please see my comments inline.
Regards,
- Vlad
On 8/17/2012 9:10 PM, Pavel Porvatov wrote:
Hi Vladislav,
I have several comments about the fix:
1. javax.swing.ToolTipManager#showTipWindow: I think toFind
variable should be initialized in if(preferredLocation != null) {
....} else {...} block. It looks more logical and code will be more
compact (adjustedPreferredLocation can be removed at all, I think)
Agreed. I've re-worked this piece.
2. Could you please explain why you are using union of all
rectangles in the getDrawingRect method? In case two monitors are
aligned on a diagonal totalRect will contain areas that are not
covered by screen devices and popups can be placed incorrectly.
Agreed again. I've re-designed getDrawingRect() function and placed
additional condition into showTipWindow() function; now every
situation should be handled correctly.
Automated test was added, please see the webrev.
Regards,
- Vlad
Regards, Pavel
Hello,
please review the fix for 7123767: Wrong tooltip location in
Multi-Monitor configurations
jdk7 webrev: http://cr.openjdk.java.net/~vkarnauk/7123767/webrev.00/
test source: http://cr.openjdk.java.net/~vkarnauk/7123767/test/
bug description:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123767
This is also a customer escalated issue in jdk6.
On multi-monitor configurations Component.getLocationOnScreen()
may return negative values. Additionally,
ToolTipManager.showTipWindow() method calculates tip window
location from Component' getLocationOnScreen() return data and
draws tip window using the same GraphicsConfiguration instance as
component's top-left corner, which again can be of negative value.
When component is stretched across multiple screen devices, this
may lead to a situation when it's tooltip is drawn on wrong monitor.
To fix this we should count all GraphicsConfiguration instances
and determine correct screen device to draw on. We should also
take into account that:
1) tip window preferred location could be set
2) preferred location could be greater than total virtual bounds
of all monitors. To make tip window visible in such case we move
it to the corresponding corner of total virtual bounds
3) exact order of monitors (left-to-right and top-to-bottom) is
unknown
I run jtreg against ToolTipManager tests and all tests passed (I
run it several times with different monitor configurations). I
also tested the fix with custom test program (please see the link
provided) with 4 possible monitor configurations (left-to-right,
right-to-left, top-to-bottom and bottom-to-top) and saw no issues.
I added no new automated test(s) because this at least would
require multiple monitors to run.
Regards,
- Vlad
#Test Results (version 2)
#Tue Sep 04 20:20:48 MSK 2012
#checksum:778207e039526833
#-----testdescription-----
$file=/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java
$root=/Users/pavel/work/jdk7u-dev/jdk/test
author=Vladislav Karnaukhov
keywords=bug7123767
run=USER_SPECIFIED main bug7123767\n
source=bug7123767.java
title=Wrong tooltip location in Multi-Monitor configurations
#-----environment-----
#-----testresult-----
description=file\:/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java
elapsed=12828 0\:00\:12.828
end=Tue Sep 04 20\:20\:48 MSK 2012
environment=regtest
execStatus=Failed. Execution failed\: `main' threw exception\:
java.lang.RuntimeException\: Tooltip window and mouse pointer are on different
GraphicsConfiguration instances
hostname=dhcp-stpetersburg-3fl-10-162-83-65.ru.oracle.com
javatestOS=Mac OS X 10.7.4 (x86_64)
javatestVersion=4.4
jtregVersion=jtreg 4.1 fcs b04
script=com.sun.javatest.regtest.RegressionScript
sections=script_messages build compile main
start=Tue Sep 04 20\:20\:35 MSK 2012
test=javax/swing/ToolTipManager/7123767/bug7123767.java
user.name=pavel
work=/Users/pavel/work/jtreg/JTwork/javax/swing/ToolTipManager/7123767
#section:script_messages
----------messages:(4/231)----------
JDK under test: (/Users/pavel/work/jdk7u-dev/build/macosx-x86_64)
openjdk version "1.7.0-internal"
OpenJDK Runtime Environment (build 1.7.0-internal-pavel_2012_08_03_12_35-b00)
OpenJDK 64-Bit Server VM (build 23.2-b09, mixed mode)
#section:build
----------messages:(3/95)----------
command: build bug7123767
reason: Named class compiled on demand
elapsed time (seconds): 2.028
result: Passed. Build successful
#section:compile
----------messages:(3/184)----------
command: compile
/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java
reason: .class file out of date or does not exist
elapsed time (seconds): 2.02
----------System.out:(0/0)----------
----------System.err:(10/777)----------
/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java:31:
warning: SunToolkit is internal proprietary API and may be removed in a future
release
import sun.awt.SunToolkit;
^
/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java:102:
warning: SunToolkit is internal proprietary API and may be removed in a future
release
SunToolkit toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
^
/Users/pavel/work/jdk7u-dev/jdk/test/javax/swing/ToolTipManager/7123767/bug7123767.java:102:
warning: SunToolkit is internal proprietary API and may be removed in a future
release
SunToolkit toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
^
3 warnings
result: Passed. Compilation successful
#section:main
----------messages:(3/108)----------
command: main bug7123767
reason: User specified action: run main bug7123767
elapsed time (seconds): 10.624
----------System.out:(0/0)----------
----------System.err:(30/2088)----------
java.lang.RuntimeException: Tooltip window and mouse pointer are on different
GraphicsConfiguration instances
at bug7123767.componentAdded(bug7123767.java:188)
at java.awt.Container.processContainerEvent(Container.java:2255)
at java.awt.Container.processEvent(Container.java:2226)
at java.awt.Component.dispatchEventImpl(Component.java:4861)
at java.awt.Container.dispatchEventImpl(Container.java:2287)
at java.awt.Component.dispatchEvent(Component.java:4687)
at java.awt.Container.addImpl(Container.java:1131)
at javax.swing.JLayeredPane.addImpl(JLayeredPane.java:230)
at java.awt.Container.add(Container.java:998)
at javax.swing.PopupFactory$LightWeightPopup.show(PopupFactory.java:754)
at javax.swing.ToolTipManager.showTipWindow(ToolTipManager.java:355)
at
javax.swing.ToolTipManager$insideTimerAction.actionPerformed(ToolTipManager.java:677)
at javax.swing.Timer.fireActionPerformed(Timer.java:312)
at javax.swing.Timer$DoPostEvent.run(Timer.java:244)
at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:251)
at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:711)
at java.awt.EventQueue.access$000(EventQueue.java:104)
at java.awt.EventQueue$3.run(EventQueue.java:672)
at java.awt.EventQueue$3.run(EventQueue.java:670)
at java.security.AccessController.doPrivileged(Native Method)
at
java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:681)
at
java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:242)
at
java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
at
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:150)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:146)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:138)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:91)
STATUS:Failed.`main' threw exception: java.lang.RuntimeException: Tooltip
window and mouse pointer are on different GraphicsConfiguration instances
result: Failed. Execution failed: `main' threw exception:
java.lang.RuntimeException: Tooltip window and mouse pointer are on different
GraphicsConfiguration instances
test result: Failed. Execution failed: `main' threw exception:
java.lang.RuntimeException: Tooltip window and mouse pointer are on different
GraphicsConfiguration instances
