Bug 216795 - css styled input text changed via js events problems - testcase
Summary: css styled input text changed via js events problems - testcase
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: unspecified
Platform: Debian testing Unspecified
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-30 12:58 UTC by Andrea Iacovitti
Modified: 2010-02-01 21:19 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
testcase (573 bytes, text/html)
2009-11-30 13:02 UTC, Andrea Iacovitti
Details
new testcase - added textarea (1.19 KB, text/html)
2010-01-24 15:36 UTC, Andrea Iacovitti
Details
screenshot of form corruption (7.44 KB, image/png)
2010-01-29 19:59 UTC, Tommi Tervo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrea Iacovitti 2009-11-30 12:58:04 UTC
Version:            (using KDE 4.3.2)
Installed from:    Debian testing/unstable Packages

I will attach a testcase through which you can check for two bugs on lineedit widget management. The tescase contains two input text boxes whose style are dinamically changed via javascript:
-The first one default to bordered red style and switch to unstyled (native widget style) when focused, then back to default style onBlur.
-The second one (on the contrary) default to native style, switch to bordered red style when focused, return to default native style onBlur.

1- Bug related to the clear button
Actually a clear button is shown on lineedit widget when it has no css border nor background image (function RenderLineEdit::setStyle in render_form.cpp). If you try to edit on the first input text, the style switches to the native one (no css border, no background image) then clear button have to be shown. Results: konqueror become semi-unresponsive and consume 100% cpu!!
Doesn't happen on the second input text box because when focused for editing the style switch to bordered, then clear button is forbidden.
For a real life test, please go to http://blogs.koolwal.net/ , on the left side there is a custom google search box (then with a background image) that switch to native when focused and shows the same problem.

2- Bug related to style update
Focus and unFocus alternately the input boxes: the styles are not correctly updated, sometime the boxes have no border at all.

Regards,
Andrea.
Comment 1 Andrea Iacovitti 2009-11-30 13:02:29 UTC
Created attachment 38709 [details]
testcase
Comment 2 Germain Garand 2010-01-22 22:12:34 UTC
SVN commit 1078782 by ggarand:

Patch by Andrea Iacovitti <aiacovitti@libero.it> :
.factor logic to determine if native widget borders should be turned off
.rename method determining if painting CSS borders is necessary
.use the former more systematically where needed
.partial fix for #216795

CCBUG:216795
CCBUG:200795

 M  +6 -6      render_form.cpp  
 M  +6 -5      render_replaced.cpp  
 M  +6 -1      render_replaced.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1078782
Comment 3 Germain Garand 2010-01-22 22:12:44 UTC
SVN commit 1078783 by ggarand:

make that restoration code a tad more thorough
(other kind of frames should be dynamically restored too)

CCBUG:216795

 M  +9 -6      render_replaced.cpp  
 M  +2 -0      render_replaced.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1078783
Comment 4 Andrea Iacovitti 2010-01-24 15:32:11 UTC
Thanks for patching, Germain.

I made some test about revision 1078783 and it seems that restoration code
doesn't work with other widget other than lineEdit.
Possible fix would be to set m_nativeFrameShape only if frame->frameShape() != QFrame::NoFrame

@@ -483,8 +483,10 @@
         // Border:
         if (QFrame* frame = qobject_cast<QFrame*>(m_widget)) {
             if (shouldDisableNativeBorders()) {
-                m_nativeFrameShape = frame->frameShape();
-                frame->setFrameShape(QFrame::NoFrame);
+                if (frame->frameShape() != QFrame::NoFrame) {
+                    m_nativeFrameShape = frame->frameShape();
+                    frame->setFrameShape(QFrame::NoFrame);
+                }
             } else if (m_nativeFrameShape != QFrame::NoFrame) {
                 frame->setFrameShape(m_nativeFrameShape);
             }

It works for me.
I will attach new testcase.

Regards,
Andrea.
Comment 5 Andrea Iacovitti 2010-01-24 15:36:47 UTC
Created attachment 40201 [details]
new testcase - added textarea
Comment 6 Germain Garand 2010-01-25 07:54:48 UTC
thanks for the neat testcase!
and patch is fine of course, good catch, I'll apply it.

While here btw, I investigated on:

> 1- Bug related to the clear button
> Actually a clear button is shown on lineedit widget when it has no css border
> nor background image (function RenderLineEdit::setStyle in render_form.cpp). If
> you try to edit on the first input text, the style switches to the native one
> (no css border, no background image) then clear button have to be shown.

in fact, it's not a style cycle. It's a cycle of, most likely, update events or maybe key events.

our widgets are a bit special because of z-order management needs...
(look for the word "redirection" in khtmlview.cpp, and KHTMLView::eventFilter for the gory details)
e.g we need to handle things like data:text/html,<input type=text><div style=background-color:blue;width:20px;height:20px;position:relative;top:-10px;left:30px>

To achieve this, we notably need to install a common eventFilter on every widget, and every widget's children to redirect all events.

We do this when receiving a QEvent::ChildPolished in KHTMLView from every newly inserted widgets.

So here the problem is that the 'clear button' child widget is created dynamically when setting isClearButtonShown(), long after its parent has been inserted in the view. So it's eventFilter never gets installed and it wreak havocs on its event handling.
Comment 7 Germain Garand 2010-01-25 10:01:20 UTC
SVN commit 1079857 by ggarand:

patch by Andrea Iacovitti to fix my flawed logic, thanks :-)

CCBUG: 216795

 M  +5 -2      render_replaced.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1079857
Comment 8 Germain Garand 2010-01-25 10:01:29 UTC
SVN commit 1079858 by ggarand:

fix event cycle happening on google-custom-search linedit widgets

BUG: 216795

 M  +8 -1      render_form.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1079858
Comment 9 Tommi Tervo 2010-01-29 19:59:37 UTC
Created attachment 40356 [details]
screenshot of form corruption

r1079858 regressed, see screenshot.
Comment 10 Germain Garand 2010-01-30 09:02:47 UTC
oh no, thanks Tommi... I forgot that findChildren is recursive, so my isWindow() check is broken, the thing gets installed on window children's children ;-(
Comment 11 Germain Garand 2010-02-01 21:18:38 UTC
SVN commit 1083713 by ggarand:

fix nasty regression noticed by Tommi.
This event filter was being installed recursively, causing linedit
completion popups to appear corrupted.

CCBUG: 216795


 M  +5 -3      render_form.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1083713
Comment 12 Germain Garand 2010-02-01 21:19:59 UTC
SVN commit 1083714 by ggarand:

Backport this nasty RC3 regression fix ;-(
----------
automatically merged revision 1083713:
fix nasty regression noticed by Tommi.
This event filter was being installed recursively, causing linedit
completion popups to appear corrupted.

CCBUG: 216795

 M  +5 -3      render_form.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1083714