From e7087c7393bfb347987766dcf07dfb9a4ce87362 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 26 Apr 2016 11:03:20 -0700 Subject: [PATCH 1/7] Fix address bar becoming unusable after going backwards or forwards The address bar dialog was being disabled when a lookup request was finished, but was not hidden - this was bug #1. Presumably, though, the intent was to have the window _not_ disappear when going backwards or forwards, but hiding the window when a lookup request finishes would cause the window to be hidden when going backwards or forwards - this was bug #2. I decided to disable the hide-on-request-finished functionality because the window is already manually hidden after entering a new address. --- interface/src/ui/AddressBarDialog.cpp | 5 ----- interface/src/ui/AddressBarDialog.h | 1 - 2 files changed, 6 deletions(-) diff --git a/interface/src/ui/AddressBarDialog.cpp b/interface/src/ui/AddressBarDialog.cpp index b483160552..ba0cf18d32 100644 --- a/interface/src/ui/AddressBarDialog.cpp +++ b/interface/src/ui/AddressBarDialog.cpp @@ -23,7 +23,6 @@ AddressBarDialog::AddressBarDialog(QQuickItem* parent) : OffscreenQmlDialog(pare auto addressManager = DependencyManager::get(); connect(addressManager.data(), &AddressManager::lookupResultIsOffline, this, &AddressBarDialog::displayAddressOfflineMessage); connect(addressManager.data(), &AddressManager::lookupResultIsNotFound, this, &AddressBarDialog::displayAddressNotFoundMessage); - connect(addressManager.data(), &AddressManager::lookupResultsFinished, this, &AddressBarDialog::hide); connect(addressManager.data(), &AddressManager::goBackPossible, this, [this] (bool isPossible) { if (isPossible != _backEnabled) { _backEnabled = isPossible; @@ -40,10 +39,6 @@ AddressBarDialog::AddressBarDialog(QQuickItem* parent) : OffscreenQmlDialog(pare _forwardEnabled = !(DependencyManager::get()->getForwardStack().isEmpty()); } -void AddressBarDialog::hide() { - ((QQuickItem*)parent())->setEnabled(false); -} - void AddressBarDialog::loadAddress(const QString& address) { qDebug() << "Called LoadAddress with address " << address; if (!address.isEmpty()) { diff --git a/interface/src/ui/AddressBarDialog.h b/interface/src/ui/AddressBarDialog.h index eab1ebae69..b2751860cc 100644 --- a/interface/src/ui/AddressBarDialog.h +++ b/interface/src/ui/AddressBarDialog.h @@ -33,7 +33,6 @@ signals: protected: void displayAddressOfflineMessage(); void displayAddressNotFoundMessage(); - void hide(); Q_INVOKABLE void loadAddress(const QString& address); Q_INVOKABLE void loadHome(); From fde1f0740cf35129c5bd7622800f20291d25ab91 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Fri, 29 Apr 2016 11:03:22 -0700 Subject: [PATCH 2/7] Fix goBackPossible not being triggered correctly in AddressManager --- libraries/networking/src/AddressManager.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 757e145c9a..3e359e5ddb 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -600,13 +600,6 @@ void AddressManager::addCurrentAddressToHistory(LookupTrigger trigger) { // if we're cold starting and this is called for the first address (from settings) we don't do anything if (trigger != LookupTrigger::StartupFromSettings) { - if (trigger == LookupTrigger::UserInput) { - // anyime the user has manually looked up an address we know we should clear the forward stack - _forwardStack.clear(); - - emit goForwardPossible(false); - } - if (trigger == LookupTrigger::Back) { // we're about to push to the forward stack // if it's currently empty emit our signal to say that going forward is now possible @@ -618,9 +611,16 @@ void AddressManager::addCurrentAddressToHistory(LookupTrigger trigger) { // and do not but it into the back stack _forwardStack.push(currentAddress()); } else { + if (trigger == LookupTrigger::UserInput) { + // anyime the user has manually looked up an address we know we should clear the forward stack + _forwardStack.clear(); + + emit goForwardPossible(false); + } + // we're about to push to the back stack - // if it's currently empty emit our signal to say that going forward is now possible - if (_forwardStack.size() == 0) { + // if it's currently empty emit our signal to say that going backward is now possible + if (_backStack.size() == 0) { emit goBackPossible(true); } From f213a059fbad6cf64be51591b44f096a7f99596c Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 4 May 2016 15:46:26 -0700 Subject: [PATCH 3/7] Fix AddressManager not always storing history when it should --- libraries/networking/src/AddressManager.cpp | 13 +++++++------ libraries/networking/src/AddressManager.h | 7 ++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 3e359e5ddb..8e8ce9c4b0 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -310,7 +310,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const if (!returnedPath.isEmpty()) { if (shouldFaceViewpoint) { // try to parse this returned path as a viewpoint, that's the only thing it could be for now - if (!handleViewpoint(returnedPath, shouldFaceViewpoint)) { + if (!handleViewpoint(returnedPath, shouldFaceViewpoint, trigger)) { qCDebug(networking) << "Received a location path that was could not be handled as a viewpoint -" << returnedPath; } @@ -446,7 +446,7 @@ bool AddressManager::handleDomainID(const QString& host) { } void AddressManager::handlePath(const QString& path, LookupTrigger trigger, bool wasPathOnly) { - if (!handleViewpoint(path, false, wasPathOnly)) { + if (!handleViewpoint(path, false, trigger, wasPathOnly)) { qCDebug(networking) << "User entered path could not be handled as a viewpoint - " << path << "- wll attempt to ask domain-server to resolve."; @@ -463,7 +463,7 @@ void AddressManager::handlePath(const QString& path, LookupTrigger trigger, bool } } -bool AddressManager::handleViewpoint(const QString& viewpointString, bool shouldFace, +bool AddressManager::handleViewpoint(const QString& viewpointString, bool shouldFace, LookupTrigger trigger, bool definitelyPathOnly, const QString& pathString) { const QString FLOAT_REGEX_STRING = "([-+]?[0-9]*\\.?[0-9]+(?:[eE][-+]?[0-9]+)?)"; const QString SPACED_COMMA_REGEX_STRING = "\\s*,\\s*"; @@ -491,8 +491,9 @@ bool AddressManager::handleViewpoint(const QString& viewpointString, bool should // before moving to a new host thanks to the information in the same lookup URL. - if (definitelyPathOnly || (!pathString.isEmpty() && pathString != _newHostLookupPath)) { - addCurrentAddressToHistory(LookupTrigger::UserInput); + if (definitelyPathOnly || (!pathString.isEmpty() && pathString != _newHostLookupPath) + || trigger == Back || trigger == Forward) { + addCurrentAddressToHistory(trigger); } if (!isNaN(newPosition.x) && !isNaN(newPosition.y) && !isNaN(newPosition.z)) { @@ -599,7 +600,7 @@ void AddressManager::copyPath() { void AddressManager::addCurrentAddressToHistory(LookupTrigger trigger) { // if we're cold starting and this is called for the first address (from settings) we don't do anything - if (trigger != LookupTrigger::StartupFromSettings) { + if (trigger != LookupTrigger::StartupFromSettings && trigger != LookupTrigger::DomainPathResponse) { if (trigger == LookupTrigger::Back) { // we're about to push to the forward stack // if it's currently empty emit our signal to say that going forward is now possible diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index c0ba69018c..9bc9bd679b 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -46,7 +46,8 @@ public: UserInput, Back, Forward, - StartupFromSettings + StartupFromSettings, + DomainPathResponse }; bool isConnected(); @@ -77,7 +78,7 @@ public slots: // we currently expect this to be called from NodeList once handleLookupString has been called with a path bool goToViewpointForPath(const QString& viewpointString, const QString& pathString) - { return handleViewpoint(viewpointString, false, false, pathString); } + { return handleViewpoint(viewpointString, false, DomainPathResponse, false, pathString); } void goBack(); void goForward(); @@ -125,7 +126,7 @@ private: bool handleNetworkAddress(const QString& lookupString, LookupTrigger trigger); void handlePath(const QString& path, LookupTrigger trigger, bool wasPathOnly = false); - bool handleViewpoint(const QString& viewpointString, bool shouldFace = false, + bool handleViewpoint(const QString& viewpointString, bool shouldFace, LookupTrigger trigger, bool definitelyPathOnly = false, const QString& pathString = QString()); bool handleUsername(const QString& lookupString); bool handleDomainID(const QString& host); From 0ec9d9e277001e55902f0476eae1339eb866a79e Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 4 May 2016 15:46:48 -0700 Subject: [PATCH 4/7] Remove goToAddressFromObject from slots --- libraries/networking/src/AddressManager.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index 9bc9bd679b..be07fc9305 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -115,8 +115,9 @@ private slots: void handleAPIResponse(QNetworkReply& requestReply); void handleAPIError(QNetworkReply& errorReply); - void goToAddressFromObject(const QVariantMap& addressMap, const QNetworkReply& reply); private: + void goToAddressFromObject(const QVariantMap& addressMap, const QNetworkReply& reply); + void setHost(const QString& host, LookupTrigger trigger, quint16 port = 0); void setDomainInfo(const QString& hostname, quint16 port, LookupTrigger trigger); From d47c74408e332b8784940a202c348d365ef1058d Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 4 May 2016 15:47:39 -0700 Subject: [PATCH 5/7] Cleanup handleViewpoint --- libraries/networking/src/AddressManager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 8e8ce9c4b0..6ef6726827 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -501,6 +501,8 @@ bool AddressManager::handleViewpoint(const QString& viewpointString, bool should QRegExp orientationRegex(QUAT_REGEX_STRING); + bool orientationChanged = false; + // we may also have an orientation if (viewpointString[positionRegex.matchedLength() - 1] == QChar('/') && orientationRegex.indexIn(viewpointString, positionRegex.matchedLength() - 1) != -1) { @@ -512,14 +514,13 @@ bool AddressManager::handleViewpoint(const QString& viewpointString, bool should if (!isNaN(newOrientation.x) && !isNaN(newOrientation.y) && !isNaN(newOrientation.z) && !isNaN(newOrientation.w)) { - emit locationChangeRequired(newPosition, true, newOrientation, shouldFace); - return true; + orientationChanged = true; } else { qCDebug(networking) << "Orientation parsed from lookup string is invalid. Will not use for location change."; } } - emit locationChangeRequired(newPosition, false, newOrientation, shouldFace); + emit locationChangeRequired(newPosition, orientationChanged, newOrientation, shouldFace); } else { qCDebug(networking) << "Could not jump to position from lookup string because it has an invalid value."; From 2dbea3769059ef7b5e85a8a9cd53c33035530847 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 16 May 2016 10:51:41 -0700 Subject: [PATCH 6/7] Fix address manager sometimes double adding address history --- libraries/networking/src/AddressManager.cpp | 45 +++++++++++++++------ libraries/networking/src/AddressManager.h | 10 +++-- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 6ef6726827..c5de5d629e 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -147,8 +147,14 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { if (!handleUsername(lookupUrl.authority())) { // we're assuming this is either a network address or global place name // check if it is a network address first + bool hostChanged; if (handleNetworkAddress(lookupUrl.host() - + (lookupUrl.port() == -1 ? "" : ":" + QString::number(lookupUrl.port())), trigger)) { + + (lookupUrl.port() == -1 ? "" : ":" + QString::number(lookupUrl.port())), trigger, hostChanged)) { + + // If the host changed then we have already saved to history + if (hostChanged) { + trigger = Internal; + } // if we were not passed a path, use the index path auto path = lookupUrl.path(); @@ -170,13 +176,13 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { } return true; + } else if (lookupUrl.toString().startsWith('/')) { qCDebug(networking) << "Going to relative path" << lookupUrl.path(); // if this is a relative path then handle it as a relative viewpoint handlePath(lookupUrl.path(), trigger, true); emit lookupResultsFinished(); - return true; } @@ -286,13 +292,19 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const const QString PLACE_ID_KEY = "id"; _rootPlaceID = rootMap[PLACE_ID_KEY].toUuid(); + bool addressChanged = false; + // set our current root place name to the name that came back const QString PLACE_NAME_KEY = "name"; QString placeName = rootMap[PLACE_NAME_KEY].toString(); if (!placeName.isEmpty()) { - setHost(placeName, trigger); + if (setHost(placeName, trigger)) { + trigger = LookupTrigger::Internal; + } } else { - setHost(domainIDString, trigger); + if (setHost(domainIDString, trigger)) { + trigger = LookupTrigger::Internal; + } } // check if we had a path to override the path returned @@ -394,7 +406,7 @@ void AddressManager::attemptDomainIDLookup(const QString& lookupString, const QS QByteArray(), NULL, requestParams); } -bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTrigger trigger) { +bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTrigger trigger, bool& hostChanged) { const QString IP_ADDRESS_REGEX_STRING = "^((?:(?:[0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}" "(?:[0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))(?::(\\d{1,5}))?$"; @@ -412,7 +424,7 @@ bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTri } emit lookupResultsFinished(); - setDomainInfo(domainIPString, domainPort, trigger); + hostChanged = setDomainInfo(domainIPString, domainPort, trigger); return true; } @@ -429,11 +441,13 @@ bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTri } emit lookupResultsFinished(); - setDomainInfo(domainHostname, domainPort, trigger); + hostChanged = setDomainInfo(domainHostname, domainPort, trigger); return true; } + hostChanged = false; + return false; } @@ -547,24 +561,27 @@ bool AddressManager::handleUsername(const QString& lookupString) { return false; } -void AddressManager::setHost(const QString& host, LookupTrigger trigger, quint16 port) { +bool AddressManager::setHost(const QString& host, LookupTrigger trigger, quint16 port) { if (host != _host || port != _port) { - _port = port; - - // if the host is being changed we should store current address in the history addCurrentAddressToHistory(trigger); + _port = port; + if (host != _host) { _host = host; emit hostChanged(_host); } + + return true; } + + return false; } -void AddressManager::setDomainInfo(const QString& hostname, quint16 port, LookupTrigger trigger) { - setHost(hostname, trigger, port); +bool AddressManager::setDomainInfo(const QString& hostname, quint16 port, LookupTrigger trigger) { + bool hostChanged = setHost(hostname, trigger, port); _rootPlaceID = QUuid(); @@ -573,6 +590,8 @@ void AddressManager::setDomainInfo(const QString& hostname, quint16 port, Lookup DependencyManager::get()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::HandleAddress); emit possibleDomainChangeRequired(hostname, port); + + return hostChanged; } void AddressManager::goToUser(const QString& username) { diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index be07fc9305..dd0dbd9f38 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -47,7 +47,8 @@ public: Back, Forward, StartupFromSettings, - DomainPathResponse + DomainPathResponse, + Internal }; bool isConnected(); @@ -118,14 +119,15 @@ private slots: private: void goToAddressFromObject(const QVariantMap& addressMap, const QNetworkReply& reply); - void setHost(const QString& host, LookupTrigger trigger, quint16 port = 0); - void setDomainInfo(const QString& hostname, quint16 port, LookupTrigger trigger); + // Set host and port, and return `true` if it was changed. + bool setHost(const QString& host, LookupTrigger trigger, quint16 port = 0); + bool setDomainInfo(const QString& hostname, quint16 port, LookupTrigger trigger); const JSONCallbackParameters& apiCallbackParameters(); bool handleUrl(const QUrl& lookupUrl, LookupTrigger trigger = UserInput); - bool handleNetworkAddress(const QString& lookupString, LookupTrigger trigger); + bool handleNetworkAddress(const QString& lookupString, LookupTrigger trigger, bool& hostChanged); void handlePath(const QString& path, LookupTrigger trigger, bool wasPathOnly = false); bool handleViewpoint(const QString& viewpointString, bool shouldFace, LookupTrigger trigger, bool definitelyPathOnly = false, const QString& pathString = QString()); From cf6eba96a4a217603e458611a22283ca1ab3fbb7 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 16 May 2016 11:20:22 -0700 Subject: [PATCH 7/7] Remove unused variable --- libraries/networking/src/AddressManager.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index c5de5d629e..27647d2694 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -292,8 +292,6 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const const QString PLACE_ID_KEY = "id"; _rootPlaceID = rootMap[PLACE_ID_KEY].toUuid(); - bool addressChanged = false; - // set our current root place name to the name that came back const QString PLACE_NAME_KEY = "name"; QString placeName = rootMap[PLACE_NAME_KEY].toString();