Review Request: Use the focus proxy, if present, as parent of KonqPopupMenu
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104890/ --- Review request for KDE Base Apps and David Faure. Description --- The following patch changes the parent widget used when creating a context menu in Konqueror. This is done to prevent unnecessary redraw that seems to occur when clicking on a kwebkitpart component that is currently displaying a flash movie. See bug#298744 for further details. What makes the problem even more confounding is the fact that if the page playing the flash movie is reloaded, either before or after clicking the RMB, then clicking the RMB button to display the context menu afterwards works just fine. It is just the first click that causes the bug. This addresses bug 298744. http://bugs.kde.org/show_bug.cgi?id=298744 Diffs - konqueror/src/konqmainwindow.cpp ea1678b Diff: http://git.reviewboard.kde.org/r/104890/diff/ Testing --- Made sure the bug reported in bug #298744 is gone after testing the patch. Thanks, Dawit Alemayehu
Re: Review Request: Use the focus proxy, if present, as parent of KonqPopupMenu
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104890/#review13602 --- This makes no sense to me. Popping up a context menu has nothing to do with keyboard focus, and the actual cause for the bug hasn't been identified, apparently. So this is a "blind workaround", which I cannot approve. - David Faure On May 9, 2012, 7:13 a.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104890/ > --- > > (Updated May 9, 2012, 7:13 a.m.) > > > Review request for KDE Base Apps and David Faure. > > > Description > --- > > The following patch changes the parent widget used when creating a context > menu in Konqueror. This is done to prevent unnecessary redraw that seems to > occur when clicking on a kwebkitpart component that is currently displaying a > flash movie. See bug#298744 for further details. > > What makes the problem even more confounding is the fact that if the page > playing the flash movie is reloaded, either before or after clicking the RMB, > then clicking the RMB button to display the context menu afterwards works > just fine. It is just the first click that causes the bug. > > > This addresses bug 298744. > http://bugs.kde.org/show_bug.cgi?id=298744 > > > Diffs > - > > konqueror/src/konqmainwindow.cpp ea1678b > > Diff: http://git.reviewboard.kde.org/r/104890/diff/ > > > Testing > --- > > Made sure the bug reported in bug #298744 is gone after testing the patch. > > > Thanks, > > Dawit Alemayehu > >
Re: Review Request: Use the focus proxy, if present, as parent of KonqPopupMenu
> On May 9, 2012, 7:37 a.m., David Faure wrote: > > This makes no sense to me. Popping up a context menu has nothing to do with > > keyboard focus, and the actual cause for the bug hasn't been identified, > > apparently. So this is a "blind workaround", which I cannot approve. Well this is most definitely not about a keyboard focus issue. Rather it is an attempt to find a way to make QWebView the parent of the popup menu instead of the KPart's widget. The KPart's widget in most cases (at least in khtml and kwebkitpart), uses the actual content viewing widget its focus proxy. That is why this patch attempts to use the focus proxy widget when available. However, you are correct. This is a workaround for a yet to be identified issue with the entire QWebView flickering and losing the flash plugin display when you attempt to popup a context menu using the KPart's widget as the parent of the popup menu. That problem does not occur if the QWebView is used as the parent widget of the context menu. No clue why this happens though. What is even more bizarre is if you simply reload the page and right click again, you will not see the flickering again. It only happens the first time the context menu is shown a page embedding a flash plugin. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104890/#review13602 --- On May 9, 2012, 7:13 a.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104890/ > --- > > (Updated May 9, 2012, 7:13 a.m.) > > > Review request for KDE Base Apps and David Faure. > > > Description > --- > > The following patch changes the parent widget used when creating a context > menu in Konqueror. This is done to prevent unnecessary redraw that seems to > occur when clicking on a kwebkitpart component that is currently displaying a > flash movie. See bug#298744 for further details. > > What makes the problem even more confounding is the fact that if the page > playing the flash movie is reloaded, either before or after clicking the RMB, > then clicking the RMB button to display the context menu afterwards works > just fine. It is just the first click that causes the bug. > > > This addresses bug 298744. > http://bugs.kde.org/show_bug.cgi?id=298744 > > > Diffs > - > > konqueror/src/konqmainwindow.cpp ea1678b > > Diff: http://git.reviewboard.kde.org/r/104890/diff/ > > > Testing > --- > > Made sure the bug reported in bug #298744 is gone after testing the patch. > > > Thanks, > > Dawit Alemayehu > >
Re: Review Request: Use the focus proxy, if present, as parent of KonqPopupMenu
> On May 9, 2012, 7:37 a.m., David Faure wrote: > > This makes no sense to me. Popping up a context menu has nothing to do with > > keyboard focus, and the actual cause for the bug hasn't been identified, > > apparently. So this is a "blind workaround", which I cannot approve. > > Dawit Alemayehu wrote: > Well this is most definitely not about a keyboard focus issue. Rather it > is an attempt to find a way to make QWebView the parent of the popup menu > instead of the KPart's widget. The KPart's widget in most cases (at least in > khtml and kwebkitpart), uses the actual content viewing widget its focus > proxy. That is why this patch attempts to use the focus proxy widget when > available. > > However, you are correct. This is a workaround for a yet to be identified > issue with the entire QWebView flickering and losing the flash plugin display > when you attempt to popup a context menu using the KPart's widget as the > parent of the popup menu. That problem does not occur if the QWebView is used > as the parent widget of the context menu. No clue why this happens though. > What is even more bizarre is if you simply reload the page and right click > again, you will not see the flickering again. It only happens the first time > the context menu is shown a page embedding a flash plugin. This is an upstream QtWebKit issue. I cannot duplicate the problem with the current trunk version. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104890/#review13602 --- On May 9, 2012, 7:13 a.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104890/ > --- > > (Updated May 9, 2012, 7:13 a.m.) > > > Review request for KDE Base Apps and David Faure. > > > Description > --- > > The following patch changes the parent widget used when creating a context > menu in Konqueror. This is done to prevent unnecessary redraw that seems to > occur when clicking on a kwebkitpart component that is currently displaying a > flash movie. See bug#298744 for further details. > > What makes the problem even more confounding is the fact that if the page > playing the flash movie is reloaded, either before or after clicking the RMB, > then clicking the RMB button to display the context menu afterwards works > just fine. It is just the first click that causes the bug. > > > This addresses bug 298744. > http://bugs.kde.org/show_bug.cgi?id=298744 > > > Diffs > - > > konqueror/src/konqmainwindow.cpp ea1678b > > Diff: http://git.reviewboard.kde.org/r/104890/diff/ > > > Testing > --- > > Made sure the bug reported in bug #298744 is gone after testing the patch. > > > Thanks, > > Dawit Alemayehu > >
