ubuntu-touch-coreapps-reviewers team mailing list archive
-
ubuntu-touch-coreapps-reviewers team
-
Mailing list archive
-
Message #05977
[Merge] lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection into lp:ubuntu-docviewer-app/reboot
Stefano Verzegnassi has proposed merging lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection into lp:ubuntu-docviewer-app/reboot.
Commit message:
* [loviewer] Adding error detection
* [loviewer] Removed updateZoomIfAutomatic() function in LOView, its code was a bit cryptic
* Added a debug option in CMakeLists
* added '*.user.*' filter in .bzrignore file
Requested reviews:
Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)
Related bugs:
Bug #1514458 in Ubuntu Document Viewer App: "Error handling missing in our libreofficekit plugin"
https://bugs.launchpad.net/ubuntu-docviewer-app/+bug/1514458
For more details, see:
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection/+merge/277295
* [loviewer] Adding error detection
* [loviewer] Removed updateZoomIfAutomatic() function in LOView, its code was a bit cryptic
* Added a debug option in CMakeLists
* added '*.user.*' filter in .bzrignore file
--
Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection into lp:ubuntu-docviewer-app/reboot.
=== modified file '.bzrignore'
--- .bzrignore 2014-10-28 22:10:16 +0000
+++ .bzrignore 2015-11-11 20:04:43 +0000
@@ -1,6 +1,7 @@
Makefile
ubuntu-docviewer-app
*.user
+*.user.*
moc_file.cpp
launcher/build-docviewer-launcher-Desktop-Debug/
launcher/build-docviewer-launcher-Desktop-Release/
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2015-10-03 12:37:24 +0000
+++ CMakeLists.txt 2015-11-11 20:04:43 +0000
@@ -14,6 +14,9 @@
set(CMAKE_AUTOMOC ON)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -fno-permissive -pedantic -Wall -Wextra -fPIC")
+# Debugging purpose. Keep commented unless you need it.
+# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_DEBUG}")
+
include(FindPkgConfig)
# Standard install paths
include(GNUInstallDirs)
=== renamed file 'src/app/qml/common/FileNotFoundDialog.qml' => 'src/app/qml/common/ErrorDialog.qml'
--- src/app/qml/common/FileNotFoundDialog.qml 2015-10-10 12:03:30 +0000
+++ src/app/qml/common/ErrorDialog.qml 2015-11-11 20:04:43 +0000
@@ -18,11 +18,9 @@
import Ubuntu.Components 1.2
import Ubuntu.Components.Popups 1.0
-// We may want to refactor this dialog for a more generic usage, when we'll need it.
Dialog {
id: errorDialog
title: i18n.tr("Error")
- text: i18n.tr("File does not exist")
Button {
text: i18n.tr("Close")
=== modified file 'src/app/qml/loView/LOViewPage.qml'
--- src/app/qml/loView/LOViewPage.qml 2015-10-19 11:44:11 +0000
+++ src/app/qml/loView/LOViewPage.qml 2015-11-11 20:04:43 +0000
@@ -164,6 +164,31 @@
loPageContent.forceActiveFocus()
}
+ onErrorChanged: {
+ var errorString;
+
+ switch(error) {
+ case LibreOffice.Error.LibreOfficeNotFound:
+ errorString = i18n.tr("LibreOffice binaries not found.")
+ break;
+ case LibreOffice.Error.LibreOfficeNotInitialized:
+ errorString = i18n.tr("Error while loading LibreOffice.")
+ break;
+ case LibreOffice.Error.DocumentNotLoaded:
+ errorString = i18n.tr("Document not loaded.")
+ break;
+ }
+
+ if (errorString) {
+ loPage.pageStack.pop()
+
+ // We create the dialog in the MainView, so that it isn't
+ // initialized by 'loPage' and keep on working after the
+ // page is destroyed.
+ mainView.showErrorDialog(errorString);
+ }
+ }
+
Scrollbar { flickableItem: loView; parent: loView.parent }
Scrollbar { flickableItem: loView; parent: loView.parent; align: Qt.AlignBottom }
}
=== modified file 'src/app/qml/ubuntu-docviewer-app.qml'
--- src/app/qml/ubuntu-docviewer-app.qml 2015-10-20 18:21:17 +0000
+++ src/app/qml/ubuntu-docviewer-app.qml 2015-11-11 20:04:43 +0000
@@ -98,6 +98,11 @@
mainView.pickMode = true
}
+ function showErrorDialog(message) {
+ PopupUtils.open(Qt.resolvedUrl("common/ErrorDialog.qml"),
+ mainView, { parent: mainView, text: message });
+ }
+
// On screen rotation, force updating of header/U8 indicators panel visibility
onIsLandscapeChanged: setHeaderVisibility(true);
@@ -122,8 +127,7 @@
onMimetypeChanged: LoadComponent.load(mimetype.name)
onErrorChanged: {
if (error == -1)
- PopupUtils.open(Qt.resolvedUrl("common/FileNotFoundDialog.qml"),
- mainView, { parent: mainView });
+ mainView.showErrorDialog(i18n.tr("File does not exist."));
}
}
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt'
--- src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt 2015-10-18 20:58:32 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt 2015-11-11 20:04:43 +0000
@@ -28,6 +28,7 @@
set(libreofficetoolkitqmlplugin_HDRS
twips.h
config.h
+ loerror.h
)
add_library(libreofficetoolkitqmlplugin MODULE
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp 2015-10-27 13:27:27 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp 2015-11-11 20:04:43 +0000
@@ -26,8 +26,6 @@
#include <LibreOfficeKit/LibreOfficeKitInit.h>
#include <LibreOfficeKit/LibreOfficeKit.hxx>
-// TODO: Error management
-
#ifdef DEBUG_TILE_BENCHMARK
#include <QElapsedTimer>
#endif
@@ -37,6 +35,7 @@
LODocument::LODocument()
: m_path("")
, m_currentPart(-1)
+ , m_error(LibreOfficeError::NoError)
, m_document(nullptr)
{
// This space is intentionally empty.
@@ -58,7 +57,7 @@
Q_EMIT pathChanged();
// Load the new document
- this->loadDocument(m_path);
+ loadDocument(m_path);
}
int LODocument::currentPart() {
@@ -78,21 +77,48 @@
}
// Load the document
-bool LODocument::loadDocument(const QString &pathName)
+void LODocument::loadDocument(const QString &pathName)
{
qDebug() << "Loading document...";
+ setError(LibreOfficeError::NoError);
if (pathName.isEmpty()) {
qDebug() << "Can't load the document, path is empty.";
- return false;
- }
-
+ return;
+ }
+
+
+ /* Get LibreOffice path */
+ const char* loPath = Config::getLibreOfficePath();
+
+ if (loPath == NULL) {
+ setError(LibreOfficeError::LibreOfficeNotFound);
+ return;
+ }
+
+
+ /* Load LibreOffice */
if (!s_office)
- s_office = lok::lok_cpp_init(Config::getLibreOfficePath(),
- Config::getLibreOfficeProfilePath());
-
+ s_office = lok::lok_cpp_init(loPath, Config::getLibreOfficeProfilePath());
+
+ if (s_office == NULL) {
+ setError(LibreOfficeError::LibreOfficeNotInitialized);
+ qDebug() << "[lok-qml]: LibreOffice not initialized.";
+ return;
+ }
+
+
+ /* Load the document */
m_document = s_office->documentLoad(m_path.toUtf8().constData());
+ if (m_document == NULL) {
+ setError(LibreOfficeError::DocumentNotLoaded);
+ qDebug() << "[lok-qml]: Document not loaded.";
+ return;
+ }
+
+
+ /* Do the further initialization */
m_docType = DocumentType(m_document->getDocumentType());
Q_EMIT documentTypeChanged();
@@ -101,7 +127,16 @@
m_document->initializeForRendering();
qDebug() << "Document loaded successfully !";
- return true;
+ return;
+}
+
+void LODocument::setError(const LibreOfficeError::Error &error)
+{
+ if (m_error == error)
+ return;
+
+ m_error = error;
+ Q_EMIT errorChanged();
}
// Return the type of the loaded document (e.g. text document,
@@ -210,7 +245,12 @@
return QString::fromUtf8(m_document->getPartName(index));
}
-
+
+LibreOfficeError::Error LODocument::error() const
+{
+ return m_error;
+}
+
// TODO: Is there some documentation on safe formats or filterOptions that can
// be used?
bool LODocument::saveAs(QString url, QString format = QString(), QString filterOptions = QString())
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.h'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.h 2015-10-04 16:11:47 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.h 2015-11-11 20:04:43 +0000
@@ -20,6 +20,8 @@
#include <QObject>
+#include "loerror.h"
+
namespace lok {
class Office;
class Document;
@@ -30,12 +32,13 @@
Q_OBJECT
Q_DISABLE_COPY(LODocument)
- Q_PROPERTY(QString path READ path WRITE setPath NOTIFY pathChanged)
- Q_PROPERTY(int currentPart READ currentPart WRITE setCurrentPart NOTIFY currentPartChanged)
+ Q_PROPERTY(QString path READ path WRITE setPath NOTIFY pathChanged)
+ Q_PROPERTY(int currentPart READ currentPart WRITE setCurrentPart NOTIFY currentPartChanged)
// Declare partsCount as constant at the moment, since LOK-plugin is just a viewer for now.
- Q_PROPERTY(int partsCount READ partsCount CONSTANT)
- Q_PROPERTY(int documentPart READ documentPart WRITE setDocumentPart NOTIFY documentPartChanged)
- Q_PROPERTY(DocumentType documentType READ documentType NOTIFY documentTypeChanged)
+ Q_PROPERTY(int partsCount READ partsCount CONSTANT)
+ Q_PROPERTY(int documentPart READ documentPart WRITE setDocumentPart NOTIFY documentPartChanged)
+ Q_PROPERTY(DocumentType documentType READ documentType NOTIFY documentTypeChanged)
+ Q_PROPERTY(LibreOfficeError::Error error READ error NOTIFY errorChanged)
Q_ENUMS(DocumentType)
public:
@@ -70,6 +73,8 @@
QString getPartName(int index) const;
void setPart(int index);
+ LibreOfficeError::Error error() const;
+
Q_INVOKABLE bool saveAs(QString url, QString format, QString filterOptions);
Q_SIGNALS:
@@ -77,13 +82,17 @@
void currentPartChanged();
void documentTypeChanged();
void documentPartChanged();
+ void errorChanged();
private:
QString m_path;
int m_currentPart;
DocumentType m_docType;
-
- bool loadDocument(const QString &pathNAme);
+ LibreOfficeError::Error m_error;
+
+ void loadDocument(const QString &pathNAme);
+
+ void setError(const LibreOfficeError::Error &error);
lok::Document *m_document;
=== added file 'src/plugin/libreofficetoolkit-qml-plugin/loerror.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loerror.h 1970-01-01 00:00:00 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loerror.h 2015-11-11 20:04:43 +0000
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef LOERROR_H
+#define LOERROR_H
+
+#include <QObject>
+
+class LibreOfficeError : public QObject
+{
+ Q_OBJECT
+ Q_ENUMS(Error)
+
+public:
+ enum Error {
+ NoError = 0,
+ LibreOfficeNotFound = 1,
+ LibreOfficeNotInitialized = 2,
+ DocumentNotLoaded = 3
+ };
+};
+
+#endif // LOERROR_H
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.cpp 2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.cpp 2015-11-11 20:04:43 +0000
@@ -40,6 +40,7 @@
, m_cacheBuffer(TILE_SIZE * 3)
, m_visibleArea(0, 0, 0, 0)
, m_bufferArea(0, 0, 0, 0)
+ , m_error(LibreOfficeError::NoError)
{
Q_UNUSED(parent)
@@ -86,9 +87,22 @@
if (m_document)
m_document->disconnect(this);
+ setError(LibreOfficeError::NoError);
+
m_document = QSharedPointer<LODocument>(new LODocument());
m_document->setPath(path);
+ /* A lot of things happens when we set the path property in
+ * m_document. Need to check if an error has been emitted. */
+ if (m_document->error() != LibreOfficeError::NoError) {
+ setError(m_document->error());
+
+ m_document.clear();
+
+ // Stop doing anything below.
+ return;
+ }
+
// TODO MOVE
m_partsModel = new LOPartsModel(m_document);
Q_EMIT partsModelChanged();
@@ -164,8 +178,16 @@
Q_EMIT cacheBufferChanged();
}
+LibreOfficeError::Error LOView::error() const
+{
+ return m_error;
+}
+
void LOView::adjustZoomToWidth()
- {
+{
+ if (!m_document)
+ return;
+
setZoomMode(LOView::FitToWidth);
zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
@@ -173,26 +195,6 @@
setZoomFactor(zoomValueToFitWidth);
qDebug() << "Adjust zoom to width - value:" << zoomValueToFitWidth;
- }
-
-bool LOView::updateZoomIfAutomatic()
-{
- // This function is only used in LOView::updateVisibleRect()
- // It returns a bool, so that we can stop the execution of that function,
- // which will be triggered again when we'll automatically update the zoom value.
- if (m_zoomMode == LOView::FitToWidth) {
- zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
- m_document->documentSize().width());
-
- if (m_zoomFactor != zoomValueToFitWidth) {
- setZoomFactor(zoomValueToFitWidth);
-
- qDebug() << "Adjust automatic zoom to width - value:" << zoomValueToFitWidth;
- return true;
- }
- }
-
- return false;
}
void LOView::updateViewSize()
@@ -210,7 +212,7 @@
void LOView::updateVisibleRect()
{
- if (!m_parentFlickable)
+ if (!m_parentFlickable || !m_document)
return;
// Changes in parentFlickable width/height trigger directly LOView::updateVisibleRect(),
@@ -222,8 +224,17 @@
// If that happens, stop the execution of this function, since the change of
// zoomFactor will trigger the updateViewSize() function, which triggers this
// function again.
- if (this->updateZoomIfAutomatic())
- return;
+ if (m_zoomMode == LOView::FitToWidth) {
+ zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
+ m_document->documentSize().width());
+
+ if (m_zoomFactor != zoomValueToFitWidth) {
+ setZoomFactor(zoomValueToFitWidth);
+
+ qDebug() << "Adjust automatic zoom to width - value:" << zoomValueToFitWidth;
+ return;
+ }
+ }
// Check if current tiles have a different zoom value
if (!m_tiles.isEmpty()) {
@@ -381,6 +392,15 @@
}
}
+void LOView::setError(const LibreOfficeError::Error &error)
+{
+ if (m_error == error)
+ return;
+
+ m_error = error;
+ Q_EMIT errorChanged();
+}
+
LOView::~LOView()
{
delete m_partsModel;
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-10-11 11:27:29 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-11-11 20:04:43 +0000
@@ -23,6 +23,7 @@
#include <QQmlContext>
#include <QQmlEngine>
+#include "loerror.h"
#include "renderengine.h"
#include "lopartsmodel.h"
#include "lopartsimageprovider.h"
@@ -34,12 +35,13 @@
{
Q_OBJECT
Q_ENUMS(ZoomMode)
- Q_PROPERTY(QQuickItem* parentFlickable READ parentFlickable WRITE setParentFlickable NOTIFY parentFlickableChanged)
- Q_PROPERTY(LODocument* document READ document /*WRITE setDocument*/ NOTIFY documentChanged)
- Q_PROPERTY(LOPartsModel* partsModel READ partsModel NOTIFY partsModelChanged)
- Q_PROPERTY(qreal zoomFactor READ zoomFactor WRITE setZoomFactor NOTIFY zoomFactorChanged)
- Q_PROPERTY(ZoomMode zoomMode READ zoomMode NOTIFY zoomModeChanged)
- Q_PROPERTY(int cacheBuffer READ cacheBuffer WRITE setCacheBuffer NOTIFY cacheBufferChanged)
+ Q_PROPERTY(QQuickItem* parentFlickable READ parentFlickable WRITE setParentFlickable NOTIFY parentFlickableChanged)
+ Q_PROPERTY(LODocument* document READ document /*WRITE setDocument*/ NOTIFY documentChanged)
+ Q_PROPERTY(LOPartsModel* partsModel READ partsModel NOTIFY partsModelChanged)
+ Q_PROPERTY(qreal zoomFactor READ zoomFactor WRITE setZoomFactor NOTIFY zoomFactorChanged)
+ Q_PROPERTY(ZoomMode zoomMode READ zoomMode NOTIFY zoomModeChanged)
+ Q_PROPERTY(int cacheBuffer READ cacheBuffer WRITE setCacheBuffer NOTIFY cacheBufferChanged)
+ Q_PROPERTY(LibreOfficeError::Error error READ error NOTIFY errorChanged)
public:
LOView(QQuickItem *parent = 0);
@@ -66,6 +68,8 @@
int cacheBuffer() const;
void setCacheBuffer(int cacheBuffer);
+ LibreOfficeError::Error error() const;
+
Q_INVOKABLE void adjustZoomToWidth();
Q_SIGNALS:
@@ -75,6 +79,7 @@
void zoomFactorChanged();
void zoomModeChanged();
void cacheBufferChanged();
+ void errorChanged();
private Q_SLOTS:
void updateViewSize();
@@ -99,6 +104,8 @@
QRect m_visibleArea;
QRect m_bufferArea;
+ LibreOfficeError::Error m_error;
+
QTimer m_updateTimer;
QMap<int, SGTileItem*> m_tiles;
@@ -106,8 +113,9 @@
void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth);
void createTile(int index, QRect rect);
void setZoomMode(const ZoomMode zoomMode);
- bool updateZoomIfAutomatic();
void clearView();
+
+ void setError(const LibreOfficeError::Error &error);
};
#endif // LOVIEW_H
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp 2015-10-05 20:53:25 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp 2015-11-11 20:04:43 +0000
@@ -22,15 +22,17 @@
#include "lodocument.h"
#include "loview.h"
#include "lopartsmodel.h"
+#include "loerror.h"
void LOPlugin::registerTypes(const char *uri)
{
Q_ASSERT(uri == QLatin1String("DocumentViewer.LibreOffice"));
//@uri DocumentViewer.LibreOffice
- qmlRegisterType<LODocument>(uri, 1, 0, "Document");
- qmlRegisterType<LOView>(uri, 1, 0, "View");
- qmlRegisterUncreatableType<LOPartsModel>(uri, 1, 0, "PartsModel", "You shouldn't create LOPartsModel in QML");
+ qmlRegisterType <LODocument> (uri, 1, 0, "Document");
+ qmlRegisterType <LOView> (uri, 1, 0, "View");
+ qmlRegisterUncreatableType <LOPartsModel> (uri, 1, 0, "PartsModel", "You shouldn't create LOPartsModel in QML");
+ qmlRegisterUncreatableType <LibreOfficeError> (uri, 1, 0, "Error", "Not creatable as an object, use only to retrieve error enums (e.g. LibreOffice.Error.DocumentNotFound)");
}
void LOPlugin::initializeEngine(QQmlEngine *engine, const char *uri)
=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml'
--- src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml 2015-10-05 06:42:33 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml 2015-11-11 20:04:43 +0000
@@ -23,8 +23,9 @@
property alias document: view.document
property alias zoomFactor: view.zoomFactor
property alias cacheBuffer: view.cacheBuffer
- property alias partsModel: view.partsModel
+ property alias partsModel: view.partsModel
property alias zoomMode: view.zoomMode
+ property alias error: view.error
property string documentPath: ""
Follow ups
