Bugzilla – Full Text Bug Listing
| Summary: | [Shell] popup shell is not restricted to the page and scrolls the whole page | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Istvan Ballok <Istvan.Ballok> | ||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | tbuschto | ||||
| Version: | 1.4 | ||||||
| Target Milestone: | 3.0 M1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Istvan Ballok
Created attachment 183242 [details] proposed fix 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) Still valid with CVS HEAD. Simple to reproduce in controls demo: -> Tab "Shell" -> "Open Shell" -> Move Shell to the bottom as far as possible. -> "Bring first Shell to top" => Page scrolls The fix should be to simply reset the scroll offset if the page scrolls, like rwt.html.Scroll.disableScrolling does. This method be used directly though because there are window, document, documentElement and body involved, and I think it depends on the browser which of these fires the scroll event and which has to reset the scroll offset (they are not always the same.) Bug 400444 could likely be fixed in the same way. Fixed in master with commit dc23ade53e74c8864d87b8144aba11ada0d01c79. |