KDE/kdelibs/khtml

[Germain Garand]

Le jeudi 18 septembre 2008, Rafael Fernández López a écrit :
> SVN commit 862207 by ereslibre:
>

Hi Rafael,
thanks for the nice events/zooming fixes!

I have some remarks about this particular commit:

[...]
> --- trunk/KDE/kdelibs/khtml/khtmlview.cpp #862206:862207
> @@ -3570,7 +3570,6 @@
>      bool altKey = (_mouse->modifiers() & Qt::AltModifier);
>      bool shiftKey = (_mouse->modifiers() & Qt::ShiftModifier);
>      bool metaKey = (_mouse->modifiers() & Qt::MetaModifier);
> -
>      // mouseout/mouseover
>      if (setUnder && d->oldUnderMouse != targetNode) {
>          if (d->oldUnderMouse && d->oldUnderMouse->document() !=
> m_part->xmlDocImpl()) { ---
> trunk/KDE/kdelibs/khtml/rendering/render_form.cpp #862206:862207 @@
> -1147,27 +1147,26 @@
>  void ComboBoxWidget::showPopup()
>  {
>      QPoint p = pos();
> -    QPoint dest;
> +    QPoint dest( p );
>      QWidget* parent = parentWidget();
>      KHTMLView* v = m_kwp->rootViewPos(dest);
>      int zoomLevel = v ? v->zoomLevel() : 100;
>      if (zoomLevel != 100) {
> -        dest.setX( dest.x()*zoomLevel/100 );
> -        dest.setY( dest.y()*zoomLevel/100 );
> +        QPoint
> zoomedOffset(m_kwp->rootViewPos(p)->horizontalScrollBar()->value()*zoomLeve
>l/100, +                           
> m_kwp->rootViewPos(p)->verticalScrollBar()->value()*zoomLevel/100); +      
>  QPoint offset(m_kwp->rootViewPos(p)->horizontalScrollBar()->value(), +    
>                  m_kwp->rootViewPos(p)->verticalScrollBar()->value()); +   

rootViewPos() might return null in some adverse conditions, so you'd better 
retrieve it once and check for nullity before dereferencing.

(also it is rather expensive, so calling it multiple times is kinda wasteful)

>     dest.setX( dest.x()*zoomLevel/100 + (zoomedOffset - offset).x() ); +   
>     // we need to place the popup even lower on the screen, take in count
> the widget is bigger +        // now, so we add also the difference between
> the original height, and the zoomed height +        dest.setY(
> dest.y()*zoomLevel/100 + ( sizeHint().height()*zoomLevel/100 -
> sizeHint().height() ) + +                   (zoomedOffset - offset).y() );
>      }


>      bool blocked = blockSignals(true);
> -    if (v != parent)
> -        setParent(v);
>      move( dest );
>      blockSignals(blocked);
>
>      KComboBox::showPopup();
> -
> -    blocked = blockSignals(true);
> -    if (v != parent)
> -        setParent(parent);
> -    move( p );
> -    blockSignals(blocked);
>  }

this bit is wrong, actually.
The reparenting is needed to translate the widget to the root View context.

In other words, if your widget is embedded within an iframe, it won't work.

Germain
P.S: I'll attach an html file I use for testing event/widgets... as the 
regression testing framework isn't really helpful for those, it is critical 
that after any change to event handling, the behaviour of all form widgets 
and iframes is tested as thoroughly as possible by interacting manually.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20080920/51adff12/attachment.html>