Download
Getting Started
Members
Projects
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
More
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
Toggle navigation
Bugzilla – Attachment 183242 Details for
Bug 330379
[Shell] popup shell is not restricted to the page and scrolls the whole page
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
Log In
[x]
|
Terms of Use
|
Copyright Agent
Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read
this important communication.
[patch]
proposed fix
330379_Shell_scroll_HEAD.patch (text/plain), 17.08 KB, created by
Istvan Ballok
on 2010-11-16 11:54:54 EST
(
hide
)
Description:
proposed fix
Filename:
MIME Type:
Creator:
Istvan Ballok
Created:
2010-11-16 11:54:54 EST
Size:
17.08 KB
patch
obsolete
>commit 3e5b48ba66e891d4602fd2fba732ea072ead885a >tree c4f335c0dac178649a47f65e153829ca751e77b2 >parent d3219a0b3cd5e1bd74d00439a51ca033591c62a2 >author Istvan Ballok <Istvan.Ballok@cas.de> 1289926342 +0100 >committer Istvan Ballok <Istvan.Ballok@cas.de> 1289926342 +0100 > > bug 330379 popup shell is not restricted to the page and scrolls the whole page > > proposed fix to the problem: > > The Problem > =========== > Open a new Shell and position it so, that it would not completely fit on > the page, e.g. by calling setLocation or making the browser window small. > > What should happen? > ------------------- > The shell should be moved in such a way, that it is fully (or partially) > visible in the browser window. > > What happens? > ------------- > - the Shell is not restricted to the page > - the whole html body is scrolled so that the newly opened shell is > fully displayed. > this is a serious problem, because the overflow for the body is hidden, > and there are no top level scrollbars. i.e., the user can not scroll > back to the top > > How to reproduce? > ----------------- > see the attached patch for the rap demo project. > > The explanation of the problem > ============================== > 1) For https://bugs.eclipse.org/bugs/show_bug.cgi?id=323730, the mechanism > to restrict the Shell to the page, on opening it, was deactivated. > I think this mechanism should be reactivated, please preferably > try to find a different resolution for bug 323730 > > 2) the last statement in the response from the RAP server in this case is: > org.eclipse.swt.WidgetManager.getInstance().focus( ... ); > > In fact, it is the Shell that gets focused. > This call executes in the end Widget.js#visualizeFocus, that calls > this.getElement().focus(); > > As we could test it in this snippet: > > <html> > <head> > <script type="text/javascript"> > function focus() { > document.getElementById("d").focus() > } > </script> > </head> > <body onload="focus()"> > <div><input tabindex="1" value="hello" type="button" /></div> > <div style="margin-top:2000px"><input tabindex="2" value="world" > type="button" id="d" /></div> > </body> > </html> > > the browser scrolls the newly focused element in view. > > The Shell.js indirectly inherits from Popup.js, that overrides the > _afterAppear function of Parent (and Widget), to first call base, and > then adjust the position of the popup, to restrict it to the page. > > First, as a result of the base call, the "appear" event is fired, > that is handled in the WidgetManager by calling focus. > > Later in _afterAppear, after some calculations, in a setTimeout call, > the new position for the shell is set. > > This means, 2 rounds of deferred execution! > 1) setTimeout call > 2) setLeft -> qooxdoo rendering queue --(deferred)-> DOM updated > > Only after these deferred calls is the DOM node of the shell in the > new position. > > If we focus it before this happens, the html body is scrolled by the > browser to display the shell, and hence we have the bug. > > In the current implementation, focus() is always called before updating the > position because the focus is called directly from _afterAppear, > and the position update takes place only after 2 deferred calls. > > Suggested fix for the problem > ============================= > The single workaround I could think of was to delay focusing the widget, > so that it happens after it is repositioned. > > Problems with the fix > --------------------- > This is unfortunately not fully reliable, because the needed delay > depends on how long it takes to execute the javascript in the deferred > execution calls + even activities in other browser tabs. > I hope you can find a better solution. : ) > > Perhaps changing the _afterAppear implementation in Popup.js to fire > the "appear" event only after the Shell is repositioned (i.e. the changes > are rendered) > >diff --git a/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/WidgetManager.js b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/WidgetManager.js >index d61a394..b283005 100644 >--- a/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/WidgetManager.js >+++ b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/WidgetManager.js >@@ -36,11 +36,14 @@ qx.Class.define( "org.eclipse.swt.WidgetManager", { > > _onAppearFocus : function( evt ) { > var widget = this; >- widget.focus(); >- evt.getTarget().removeEventListener( >- "appear", >- org.eclipse.swt.WidgetManager._onAppearFocus, >- widget ); >+ var target = evt.getTarget(); >+ setTimeout(function(){ >+ widget.focus(); >+ target.removeEventListener( >+ "appear", >+ org.eclipse.swt.WidgetManager._onAppearFocus, >+ widget ); >+ }, 100); > } > }, > >diff --git a/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Shell.js b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Shell.js >index 4c2c221..2cedf19 100644 >--- a/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Shell.js >+++ b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Shell.js >@@ -17,7 +17,7 @@ qx.Class.define( "org.eclipse.swt.widgets.Shell", { > this.base( arguments ); > this.setOverflow( qx.constant.Style.OVERFLOW_HIDDEN ); > // Note: This prevents a laoyut-glitch on the ipad: >- this.setRestrictToPageOnOpen( false ); >+ //this.setRestrictToPageOnOpen( false ); > // TODO [rh] HACK to set mode on Label that shows the caption, _captionTitle > // is a 'protected' field on class Window > this._captionTitle.setMode( "html" ); > >commit d3219a0b3cd5e1bd74d00439a51ca033591c62a2 >tree 406803f431abe46bc96aebca462cd126fc8be24d >parent 98472489803523cf0e3b3c489e575eea8aa168d4 >author Istvan Ballok <Istvan.Ballok@cas.de> 1289926074 +0100 >committer Istvan Ballok <Istvan.Ballok@cas.de> 1289926074 +0100 > > bug 330379 popup shell is not restricted to the page and scrolls the whole page > > simple change to reproduce the problem: > > The Problem > =========== > Open a new Shell and position it so, that it would not completely fit on > the page, e.g. by calling setLocation or making the browser window small. > > What should happen? > ------------------- > The shell should be moved in such a way, that it is fully (or partially) > visible in the browser window. > > What happens? > ------------- > - the Shell is not restricted to the page > - the whole html body is scrolled so that the newly opened shell is > fully displayed. > this is a serious problem, because the overflow for the body is hidden, > and there are no top level scrollbars. i.e., the user can not scroll > back to the top > > How to reproduce? > ----------------- > see the attached patch for the rap demo project. > >diff --git a/runtime.ui/org.eclipse.rap.demo/src/org/eclipse/rap/demo/controls/ShellTab.java b/runtime.ui/org.eclipse.rap.demo/src/org/eclipse/rap/demo/controls/ShellTab.java >index 2e63c0d..15054ec 100644 >--- a/runtime.ui/org.eclipse.rap.demo/src/org/eclipse/rap/demo/controls/ShellTab.java >+++ b/runtime.ui/org.eclipse.rap.demo/src/org/eclipse/rap/demo/controls/ShellTab.java >@@ -349,7 +349,7 @@ public class ShellTab extends ExampleTab { > Point result = getShell().getLocation(); > int count = getShells().length % 12; > result.x += 50 + count * 10; >- result.y += 50 + count * 10; >+ result.y += 650 + count * 10; > return result ; > } > > >commit 98472489803523cf0e3b3c489e575eea8aa168d4 >tree 68a3d3e2ddb1a9974c6a9fb7504cef287e9fbacb >parent d5f6cd8c4d0173db5bda0344c30104d7f2c387e5 >author Istvan Ballok <Istvan.Ballok@cas.de> 1289926020 +0100 >committer Istvan Ballok <Istvan.Ballok@cas.de> 1289926020 +0100 > > fix git mirror of the rap cvs repository - add missing Scrollable.js file > > unfortunately: > https://bugs.eclipse.org/bugs/show_bug.cgi?id=330271 > >diff --git a/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Scrollable.js b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Scrollable.js >new file mode 100755 >index 0000000..9963306 >--- /dev/null >+++ b/runtime.rwt/org.eclipse.rap.rwt.q07/js/org/eclipse/swt/widgets/Scrollable.js >@@ -0,0 +1,245 @@ >+/******************************************************************************* >+ * Copyright: 2004-2010 1&1 Internet AG, Germany, http://www.1und1.de, >+ * and EclipseSource >+ * >+ * This program and the accompanying materials are made available under the >+ * terms of the Eclipse Public License v1.0 which accompanies this >+ * distribution, and is available at http://www.eclipse.org/legal/epl-v10.html >+ * >+ * Contributors: >+ * 1&1 Internet AG and others - original API and implementation >+ * EclipseSource - adaptation for the Eclipse Rich Ajax Platform >+ ******************************************************************************/ >+ >+qx.Class.define( "org.eclipse.swt.widgets.Scrollable", { >+ extend : qx.ui.layout.CanvasLayout, >+ >+ construct : function( clientArea ) { >+ this.base( arguments ); >+ this._clientArea = clientArea; >+ this._horzScrollBar = new qx.ui.basic.ScrollBar( true ); >+ this._vertScrollBar = new qx.ui.basic.ScrollBar( false ); >+ this._blockScrolling = false; >+ this._internalChangeFlag = false; >+ this.add( this._clientArea ); >+ this.add( this._horzScrollBar ); >+ this.add( this._vertScrollBar ); >+ this._configureScrollBars(); >+ this._configureClientArea(); >+ this.__onscroll = qx.lang.Function.bindEvent( this._onscroll, this ); >+ }, >+ >+ destruct : function() { >+ var el = this._clientArea._getTargetNode(); >+ if( el ) { >+ var eventUtil = qx.html.EventRegistration; >+ eventUtil.removeEventListener( el, "scroll", this.__onscroll ); >+ delete this.__onscroll; >+ } >+ this._clientArea = null; >+ this._horzScrollBar = null; >+ this._vertScrollBar = null; >+ }, >+ >+ events : { >+ "userScroll" : "qx.event.type.Event" >+ }, >+ >+ members : { >+ >+ ///////// >+ // Public >+ >+ setScrollBarsVisible : function( horizontal, vertical ) { >+ this._horzScrollBar.setDisplay( horizontal ); >+ this._vertScrollBar.setDisplay( vertical ); >+ var overflow = "hidden"; >+ if( horizontal && vertical ) { >+ overflow = "scroll"; >+ } else if( horizontal ) { >+ overflow = "scrollX"; >+ } else if( vertical ) { >+ overflow = "scrollY"; >+ } >+ this._clientArea.setOverflow( overflow ); >+ this._layoutX(); >+ this._layoutY(); >+ }, >+ >+ setHBarSelection : function( value ) { >+ this._internalChangeFlag = true; >+ if( this._horzScrollBar.getMaximum() < value ) { >+ // TODO [tb] : The ScrollBar should do that itself >+ this._horzScrollBar.setMaximum( value ); >+ } >+ this._horzScrollBar.setValue( value ); >+ this._internalChangeFlag = false; >+ }, >+ >+ setVBarSelection : function( value ) { >+ this._internalChangeFlag = true; >+ if( this._vertScrollBar.getMaximum() < value ) { >+ // TODO [tb] : The ScrollBar should do that itself >+ this._vertScrollBar.setMaximum( value ); >+ } >+ this._vertScrollBar.setValue( value ); >+ this._internalChangeFlag = false; >+ }, >+ >+ setBlockScrolling : function( value ) { >+ this._blockScrolling = value; >+ }, >+ >+ ///////// >+ // Layout >+ >+ _configureClientArea : function() { >+ this._clientArea.setOverflow( "scroll" ); >+ this._clientArea.setLeft( 0 ); >+ this._clientArea.setTop( 0 ); >+ this._clientArea.addEventListener( "create", this._onClientCreate, this ); >+ this._clientArea.addEventListener( "appear", this._onClientAppear, this ); >+ // TOOD [tb] : Do this with an eventlistner after fixing Bug 327023 >+ this._clientArea._layoutPost >+ = qx.lang.Function.bindEvent( this._onClientLayout, this ); >+ }, >+ >+ _configureScrollBars : function() { >+ var dragBlocker = function( event ) { event.stopPropagation(); }; >+ var preferredWidth = this._vertScrollBar.getPreferredBoxWidth(); >+ var preferredHeight = this._horzScrollBar.getPreferredBoxHeight(); >+ this._horzScrollBar.setLeft( 0 ); >+ this._horzScrollBar.setHeight( preferredHeight ); >+ this._horzScrollBar.addEventListener( "dragstart", dragBlocker ); >+ this._vertScrollBar.setTop( 0 ); >+ this._vertScrollBar.setWidth( preferredWidth ); >+ this._vertScrollBar.addEventListener( "dragstart", dragBlocker ); >+ this._horzScrollBar.addEventListener( "changeValue", >+ this._onHorzScrollBarChangeValue, >+ this ); >+ this._vertScrollBar.addEventListener( "changeValue", >+ this._onVertScrollBarChangeValue, >+ this ); >+ }, >+ >+ _applyWidth : function( newValue, oldValue ) { >+ this.base( arguments, newValue, oldValue ); >+ this._layoutX(); >+ }, >+ >+ _applyHeight : function( newValue, oldValue ) { >+ this.base( arguments, newValue, oldValue ); >+ this._layoutY(); >+ }, >+ >+ _applyBorder : function( newValue, oldValue ) { >+ this.base( arguments, newValue, oldValue ); >+ this._layoutX(); >+ this._layoutY(); >+ }, >+ >+ _layoutX : function() { >+ var clientWidth = this.getWidth() - this.getFrameWidth(); >+ if( this._vertScrollBar.getDisplay() ) { >+ clientWidth -= this._vertScrollBar.getWidth(); >+ } >+ this._clientArea.setWidth( clientWidth ); >+ this._vertScrollBar.setLeft( clientWidth ); >+ this._horzScrollBar.setWidth( clientWidth ); >+ }, >+ >+ _layoutY : function() { >+ var clientHeight = this.getHeight() - this.getFrameHeight(); >+ if( this._horzScrollBar.getDisplay() ) { >+ clientHeight -= this._horzScrollBar.getHeight(); >+ } >+ this._clientArea.setHeight( clientHeight ); >+ this._vertScrollBar.setHeight( clientHeight ); >+ this._horzScrollBar.setTop( clientHeight ); >+ }, >+ >+ _onClientCreate : function( evt ) { >+ this._clientArea.prepareEnhancedBorder(); >+ var el = this._clientArea._getTargetNode(); >+ var eventUtil = qx.html.EventRegistration; >+ eventUtil.addEventListener( el, "scroll", this.__onscroll ); >+ }, >+ >+ _onClientLayout : function() { >+ // TODO [tb] : _getScrollBarWidth should be a static function, it has >+ // nothing to do with the instance, which here is only used to call it >+ var barWidth = this._horzScrollBar._getScrollBarWidth(); >+ var node = this._clientArea._getTargetNode(); >+ var el = this._clientArea.getElement(); >+ var overflow = this._clientArea.getOverflow(); >+ var width = parseInt( el.style.width ); >+ var height = parseInt( el.style.height ); >+ if( overflow === "scroll" || overflow === "scrollY" ) { >+ width += barWidth; >+ } >+ if( overflow === "scroll" || overflow === "scrollX" ) { >+ height += barWidth; >+ } >+ node.style.width = width >+ node.style.height = height; >+ }, >+ >+ //////////// >+ // Scrolling >+ >+ _onHorzScrollBarChangeValue : function() { >+ if( this._isCreated ) { >+ this._syncClientArea( true, false ); >+ } >+ if( !this._internalChangeFlag ) { >+ this.createDispatchEvent( "userScroll" ); >+ } >+ }, >+ >+ _onVertScrollBarChangeValue : function() { >+ if( this._isCreated ) { >+ this._syncClientArea( false, true ); >+ } >+ if( !this._internalChangeFlag ) { >+ this.createDispatchEvent( "userScroll" ); >+ } >+ }, >+ >+ _onClientAppear : function() { >+ this._internalChangeFlag = true; >+ this._syncClientArea( true, true ); >+ this._internalChangeFlag = false; >+ }, >+ >+ _onscroll : function( evt ) { >+ if( !this._internalChangeFlag ) { >+ org.eclipse.rwt.EventHandlerUtil.stopDomEvent( evt ); >+ var blockH = this._blockScrolling || !this._horzScrollBar.getDisplay(); >+ var blockV = this._blockScrolling || !this._vertScrollBar.getDisplay(); >+ this._internalChangeFlag = true; >+ this._syncClientArea( blockH, blockV ); >+ this._internalChangeFlag = false; >+ this._syncScrollBars(); >+ } >+ }, >+ >+ _syncClientArea : function( horz, vert ) { >+ if( horz ) { >+ var scrollX = this._horzScrollBar.getValue(); >+ this._clientArea.setScrollLeft( scrollX ); >+ } >+ if( vert ) { >+ var scrollY = this._vertScrollBar.getValue(); >+ this._clientArea.setScrollTop( scrollY ); >+ } >+ }, >+ >+ _syncScrollBars : function() { >+ var scrollX = this._clientArea.getScrollLeft(); >+ this._horzScrollBar.setValue( scrollX ); >+ var scrollY = this._clientArea.getScrollTop(); >+ this._vertScrollBar.setValue( scrollY ); >+ } >+ >+ } >+} );
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 330379
: 183242