StandardDocumentationView positioning fixes
ClosedPublic

Authored by antonanikin on Dec 15 2016, 6:45 AM.

Details

Summary

The patch fixes wrong positioning for StandardDocumentationView. Steps to reproduce:

  1. Place such test code into editor:
#include <QWebView>
#include <QLabel>
#include <QDebug>

void test()
{
    QWebView v;
    v.setUrl(QUrl("https://www.kdevelop.org/"));

    QLabel l;
    l.setText("label_text");

    qDebug() << "debug_message";
}
  1. Move cursor to any method and call context help.
  2. Help view will be shown with wrong position.
  3. Recall context help for previous place.
  4. Help view will be shown with right position.
  5. At any help view show we will see fast scrollbar "jumping" from initial (0) to some final position.

The cause of the problem is:

  1. Some page is loaded and loadFinished() signal is emitted, then QWebView sets right position inside page.
  1. After loadFinished() emitting, page JS code finishes it's work and changes font settings (size). This leads to page contents "moving" inside view widget and as a result we have wrong position.

To fix this, first, we disable view painter updates during load to avoid content "flickering" and also to hide font size "jumping". Secondly, we reset position inside page after loading with using standard QWebFrame method scrollToAnchor().

Test Plan

Tested on master branch.

Diff Detail

Repository
R33 KDevPlatform
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antonanikin updated this revision to Diff 9027.Dec 15 2016, 6:45 AM
antonanikin retitled this revision from to StandardDocumentationView positioning fixes.
antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)
antonanikin added a reviewer: KDevelop.
antonanikin added a subscriber: kdevelop-devel.
antonanikin updated this object.Dec 15 2016, 6:47 AM
antonanikin updated this revision to Diff 9028.Dec 15 2016, 7:07 AM

small fixes

kfunk requested changes to this revision.Dec 17 2016, 12:01 AM
kfunk added a reviewer: kfunk.
kfunk added a subscriber: kfunk.

This leads to rendering artifacts here (Qt 5.6). When I try to open the documentation of a symbol, I get nothing displayed (see screenshot).

documentation/standarddocumentationview.cpp
27

Unused include

52

No longer needed afaics, has been introduced with a108066e644861b2756009b57c02d4b42792fead.

105
This revision now requires changes to proceed.Dec 17 2016, 12:01 AM

This leads to rendering artifacts here (Qt 5.6). When I try to open the documentation of a symbol, I get nothing displayed (see screenshot).

D3671 was applied (it fixes loadFinished() emitting for QtHelp)? If not, painting will be broken.

antonanikin updated this revision to Diff 9106.Dec 17 2016, 6:56 AM
antonanikin edited edge metadata.

code simplify

antonanikin updated this revision to Diff 9107.Dec 17 2016, 6:58 AM
antonanikin edited edge metadata.

unused include remove

antonanikin marked 3 inline comments as done.Dec 17 2016, 7:02 AM
antonanikin added inline comments.
documentation/standarddocumentationview.cpp
52

Fixed, now we don't use JS for page scrolling.

105

Damn! How could I not notice this method??!

Thanks a lot!

antonanikin updated this object.Dec 17 2016, 7:04 AM
antonanikin updated this revision to Diff 9108.Dec 17 2016, 7:07 AM
antonanikin marked 2 inline comments as done.
antonanikin updated this object.

code simplify

kfunk accepted this revision.Dec 17 2016, 11:53 AM
kfunk edited edge metadata.

Rest LGTM.

Comments about my remark around the event filter?

documentation/standarddocumentationview.cpp
84

I'm not sure we want to adjust the position on resize. I would just drop that whole event filter.

Steps to reproduce where this feature looks odd:

  • Scroll to random position in the documentation view
  • Resize the tool view

Actual:

  • View gets scrolled back to the last looked up item

Expected:

  • Shouldn't scroll
This revision is now accepted and ready to land.Dec 17 2016, 11:53 AM

Comments about my remark around the event filter?

Ok, resize handler will be removed. I agree, new behavior is odd in your case, but standard behavior is also odd during widget resize :)

antonanikin updated this revision to Diff 9114.Dec 17 2016, 1:37 PM
antonanikin edited edge metadata.

remove resize hander; code simplify

antonanikin updated this revision to Diff 9115.Dec 17 2016, 1:40 PM
antonanikin marked an inline comment as done.

code cleanup

antonanikin updated this object.Dec 17 2016, 1:43 PM
antonanikin edited the test plan for this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.