D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela abandoned this revision.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: alex, meven, anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Luigi Toscano]

ltoscano added a comment.


  Laurent kindly took over this, extended and updated this as part of 
https://invent.kde.org/system/khelpcenter/-/merge_requests/25 (see 
https://invent.kde.org/system/khelpcenter/-/merge_requests/25/diffs?commit_id=f6d308672a9f68c20585f5d05b6c6032355f850a
 ) and it was merged in the kf6 branch. This review can be closed.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: alex, meven, anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Alexander Lohnau]

alex added a comment.


  @meven Would you be interested in taking this over? :)

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: alex, meven, anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Méven Car]

meven added a comment.


  In D23866#557830 , @svuorela wrote:
  
  > In D23866#553108 , @mlaurent 
wrote:
  >
  > > Ping ? What is the status ?:)
  >
  >
  > I'll be happy if someone else takes over. I'm still a bit hung up on other 
stuff. Don't expect much from me on this before christmas.
  >  /Sune
  
  
  How about now ?

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: meven, anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela added a comment.


  In D23866#553108 , @mlaurent wrote:
  
  > Ping ? What is the status ?:)
  
  
  I'll be happy if someone else takes over. I'm still a bit hung up on other 
stuff. Don't expect much from me on this before christmas.
  /Sune

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Laurent Montel]

mlaurent added a comment.


  Ping ? What is the status ?:)

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Luigi Toscano]

ltoscano added a comment.


  Technically I think that an hard dependency on QtWebEngine is a mistake, and 
that both engines should be available (QTextBrowser too). 
  Sune thinks that this would be not wise. Oh, well. As I don't want to block 
this right now (but it can be changed later) even if I don't agree, if it's 
possible to not have regressions (apart maybe the "move to another page with 
space"), feel free to move forward (I won't be around in the next few days).

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D23866#535801 , @svuorela wrote:
  
  > I have played a bit around with QTextBrowser, but couldn't get a working 
result yet.
  
  
  I don't want to block the patch, it was a thinking not a real investigation 
in code base.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela added a comment.


  In D23866#535611 , @mlaurent wrote:
  
  > So what is the status of this patch ?
  
  
  I have played a bit around with QTextBrowser, but couldn't get a working 
result yet.
  
  And life kind of happened.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Laurent Montel]

mlaurent added a comment.


  So what is the status of this patch ?

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Anthony Fieroni]

anthonyfieroni added a comment.


  I think QTextBrowser fits better than WebEngine.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: anthonyfieroni, kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Laurent Montel]

mlaurent added a comment.


  You removed font settings dialog.
  But we can add default font as it:
  
const QFontInfo 
font(QFontDatabase().systemFont(QFontDatabase::GeneralFont));
settings()->setFontFamily(QWebEngineSettings::StandardFont, font.family());
settings()->setFontSize(QWebEngineSettings::DefaultFontSize, 
font.pixelSize());

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Laurent Montel]

mlaurent requested changes to this revision.
mlaurent added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> mainwindow.cpp:263
> -action->setIcon( QIcon::fromTheme( QStringLiteral("zoom-in") ) );
> -connect( action, &QAction::triggered, this, 
> &MainWindow::slotIncFontSizes );
> -

We still have zoom support in webengine. see setZoomFactor
I think that it's useful to be able to increase/decrease it.

> mainwindow.cpp:292
>  
> -void MainWindow::print()
>  {

Print support can be implemented

I use a code as it:
bool WebEnginePage::execPrintPreviewPage(QPrinter *printer, int timeout)
{

  QPointer loop = new QEventLoop;
  bool result = false;
  QTimer::singleShot(timeout, loop.data(), &QEventLoop::quit);
  
  print(printer, [loop, &result](bool res) {
  if (loop && loop->isRunning()) {
  result = res;
  loop->quit();
  }
  });
  
  loop->exec();
  delete loop;
  
  return result;

}
see WebEnginePage

> view.cpp:213
> -
> -void View::showMenu( const QString& url, const QPoint& pos)
> -{

QAction *QWebEngineView::pageAction(QWebEnginePage::WebAction action) const
can generate some actions.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[David Faure]

dfaure added a comment.


  And a lot of the code in the webengine part is about cookies handling, wallet 
handling, SSL handling, all irrelevant for KHelpCenter.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela added a comment.


  In D23866#529334 , @ltoscano wrote:
  
  > Thanks, you have been fast. Would it be possible to use the 
QWebEngine-based KPart currently in konqueror.git?
  
  
  I'm not sure it makes much sense - we aren't doing kpart loading, just 
embedding a widgett.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela added a reviewer: mlaurent.
svuorela added a comment.


  Adding Montel because he has experience with locking QtWE down for e.g. mail 
usage.

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr, mlaurent
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Luigi Toscano]

ltoscano added a comment.


  Thanks, you have been fast. Would it be possible to use the QWebEngine-based 
KPart currently in konqueror.git?

REPOSITORY
  R125 KHelpCenter

REVISION DETAIL
  https://phabricator.kde.org/D23866

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela updated this revision to Diff 65825.
svuorela added a comment.


  Forgot some removed files

REPOSITORY
  R125 KHelpCenter

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23866?vs=65824&id=65825

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23866

AFFECTED FILES
  CMakeLists.txt
  bookmarkowner.cpp
  fontdialog.cpp
  fontdialog.h
  glossary.cpp
  history.cpp
  mainwindow.cpp
  mainwindow.h
  navigator.cpp
  searchengine.cpp
  view.cpp
  view.h

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr
Cc: kde-doc-english, gennad, fbampaloukas, skadinna

D23866: Port from KHTML to QtWebEngine

[Sune Vuorela]

svuorela created this revision.
svuorela added reviewers: vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, 
lueck, clauschristensen, cordlandwehr.
Herald added a project: Documentation.
Herald added a subscriber: kde-doc-english.
svuorela requested review of this revision.

REVISION SUMMARY
  Still relies on the man and help kioslaves.
  
  Features lost:
  
  - printing
  - the abitility to trigger next/previous chapter from the shell
  - the ability to with space automatic switch chapter
  - custom right click menues. The QtWE probably fits us
  
  This kind of fixes T11541 

TEST PLAN
  Clicked a bit around, things kind of work. But there are likely bugs still.
  
  The reviewers are some dinosaurs to have a chance of finding errors, and 
someone my memory identifies as documentation people.

REPOSITORY
  R125 KHelpCenter

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23866

AFFECTED FILES
  CMakeLists.txt
  bookmarkowner.cpp
  glossary.cpp
  history.cpp
  mainwindow.cpp
  mainwindow.h
  navigator.cpp
  searchengine.cpp
  view.cpp
  view.h

To: svuorela, vkrause, dfaure, carewolf, pino, ltoscano, lbeltrame, lueck, 
clauschristensen, cordlandwehr
Cc: kde-doc-english, gennad, fbampaloukas, skadinna