Repository: spyder-ide/spyder
Pull request: PR: Fix link handling and pydoc numpy related import for Online Help
<!--- Make sure to read the Contributing Guidelines:                   --->
<!--- https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md --->
<!--- and follow PEP 8, PEP 257 and Spyder's code style:               --->
<!--- https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style      --->

## Description of Changes


* [x] Wrote at least one-line docstrings (for any new functions)
* [x] Added unit test(s) covering the changes (if testable)
<!--- Remember that an image/animation is worth a thousand words! --->
* [x] Included a screenshot or animation (if affecting the UI, see [Licecap](https://www.cockos.com/licecap/))


<!--- Explain what you've done and why --->

This fixes the error when clicking the available links. Since the reimplementation of `acceptNavigationRequest` caused a trigger of the `loadFinished` signal with a `False` value (meaning that the page didn't loaded and causing the render of the page saying ``)

A preview:
![oh](https://user-images.githubusercontent.com/16781833/69109491-f7860c80-0a45-11ea-92f6-2ffaf0b4c35a.gif)

![image](https://user-images.githubusercontent.com/16781833/69119190-cae0ed80-0a63-11ea-8493-c320e2b8603e.png)


### Issue(s) Resolved

<!--- List the issue(s) below, in the form "Fixes #1234"; one per line --->

Fixes #5533 

### Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the [Developer Certificate of Origin](https://developercertificate.org)
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

<!--- TYPE YOUR USER/NAME AFTER THE FOLLOWING: --->
I certify the above statement is true and correct: dalthviz

<!--- Thanks for your help making Spyder better for everyone! --->


Problem statement:
Online help index contains invalid links
## Description

**What steps will reproduce the problem?**

1. Select Online Help
2. From the "pinky" index page select any topic or link
3. "Unable to load page" is displayed and "lank" is inserted in Module or Package text box

**What is the expected output? What do you see instead?**
Load and display the selected topic's html page

**Please provide any additional information below**

----------------------------------------


## Version and main components

* Spyder Version: 3.2.4 
* Python Version: 3.6.3
* Qt Versions: 5.9.1, PyQt5 5.9 on Darwin

## Dependencies
```
pyflakes >=0.6.0 :  1.6.0 (OK)
pycodestyle >=2.3:  2.3.1 (OK)
pygments >=2.0   :  2.2.0 (OK)
pandas >=0.13.1  :  None (NOK)
numpy >=1.7      :  1.13.3 (OK)
sphinx >=0.6.6   :  1.6.4 (OK)
rope >=0.9.4     :  0.10.7 (OK)
jedi >=0.9.0     :  0.11.0 (OK)
psutil >=0.3     :  5.4.0 (OK)
nbconvert >=4.0  :  5.3.1 (OK)
sympy >=0.7.3    :  None (NOK)
cython >=0.21    :  None (NOK)
qtconsole >=4.2.0:  4.3.1 (OK)
IPython >=4.0    :  6.2.1 (OK)
pylint >=0.25    :  1.7.4 (OK)

```

Hints from issue discussion:
Sorry, this is not a Spyder issue. The only thing we do is to render what the `pydoc` module provides.
@ccordoba12 I'm seeing this as well; the issue isn't necessarily "invalid internal links" but that clicking any link, on any page, doesn't work and produces the text ``lank`` in the search bar while entering the name directly in the search box does. I tested in Spyder 3.2.6-3.3.3 and Python 3.6.5-3.7.3 in several different environments and all of them produced the error. I can't test in Spyder 4 because ``View > Panes > Online Help`` is missing, even though ``spyder/plugins/onlinehelp`` is still present in the codebase.

Commit message: Online Help: Skip test on MacOS-PY3. Add numpy.core to direct import mods

Patch to review:
diff --git a/spyder/plugins/onlinehelp/tests/test_pydocgui.py b/spyder/plugins/onlinehelp/tests/test_pydocgui.py
index 23b3b795d78..39cd8073910 100644
--- a/spyder/plugins/onlinehelp/tests/test_pydocgui.py
+++ b/spyder/plugins/onlinehelp/tests/test_pydocgui.py
@@ -7,12 +7,15 @@
 """
 Tests for pydocgui.py
 """
+# Standard library imports
+import sys
 
 # Test library imports
 import pytest
 
 # Local imports
 from spyder.plugins.onlinehelp.widgets import PydocBrowser
+from spyder.py3compat import PY3
 
 
 @pytest.fixture
@@ -20,12 +23,38 @@ def pydocbrowser(qtbot):
     """Set up pydocbrowser."""
     widget = PydocBrowser(None)
     qtbot.addWidget(widget)
-    return widget
+    return qtbot, widget
 
 
 def test_pydocbrowser(pydocbrowser):
     """Run Pydoc Browser."""
-    assert pydocbrowser
+    qtbot, browser = pydocbrowser
+    assert browser
+
+
+@pytest.mark.parametrize(
+    "lib", [('str', 'class str', 1),
+            ('numpy.compat', 'numpy.compat', 2)
+            ])
+@pytest.mark.skipif(sys.platform == 'darwin' and PY3,
+                    reason="Times out on macOS with Python 3")
+def test_get_pydoc(pydocbrowser, lib):
+    """
+    Go to the documentation by url.
+    Regression test for spyder-ide/spyder#10740
+    """
+    qtbot, browser = pydocbrowser
+    element, doc, matches = lib
+    webview = browser.webview
+    with qtbot.waitSignal(webview.loadFinished, timeout=6000):
+        browser.initialize()
+    element_url = browser.text_to_url(element)
+    with qtbot.waitSignal(webview.loadFinished):
+        browser.set_url(element_url)
+    # Check number of matches. In Python 2 are 3 matches instead
+    # of 2 for numpy.compat
+    qtbot.waitUntil(
+        lambda: webview.get_number_matches(doc) in [matches, matches + 1])
 
 
 if __name__ == "__main__":
diff --git a/spyder/plugins/onlinehelp/widgets.py b/spyder/plugins/onlinehelp/widgets.py
index 89ff9bfd958..48b27dae5b3 100644
--- a/spyder/plugins/onlinehelp/widgets.py
+++ b/spyder/plugins/onlinehelp/widgets.py
@@ -8,6 +8,7 @@
 
 # Standard library imports
 import os.path as osp
+import pydoc
 import sys
 
 # Third party imports
@@ -22,6 +23,189 @@
 from spyder.widgets.browser import WebBrowser
 
 
+# =============================================================================
+# Pydoc adjustments
+# =============================================================================
+# This is needed to prevent pydoc raise an ErrorDuringImport when
+# trying to import numpy.
+# See spyder-ide/spyder#10740
+DIRECT_PYDOC_IMPORT_MODULES = ['numpy', 'numpy.core']
+try:
+    from pydoc import safeimport
+
+    def spyder_safeimport(path, forceload=0, cache={}):
+        if path in DIRECT_PYDOC_IMPORT_MODULES:
+            forceload = 0
+        return safeimport(path, forceload=forceload, cache=cache)
+
+    pydoc.safeimport = spyder_safeimport
+except Exception:
+    pass
+
+
+# A reimplementation of pydoc._start_server to handle connection errors
+# for 'do_GET'.
+def _start_server(urlhandler, hostname, port):
+    """Start an HTTP server thread on a specific port.
+
+    Taken from PyDoc: https://github.com/python/cpython/blob/3.7/Lib/pydoc.py
+    Start an HTML/text server thread, so HTML or text documents can be
+    browsed dynamically and interactively with a Web browser.  Example use:
+
+        >>> import time
+        >>> import pydoc
+
+        Define a URL handler.  To determine what the client is asking
+        for, check the URL and content_type.
+
+        Then get or generate some text or HTML code and return it.
+
+        >>> def my_url_handler(url, content_type):
+        ...     text = 'the URL sent was: (%s, %s)' % (url, content_type)
+        ...     return text
+
+        Start server thread on port 0.
+        If you use port 0, the server will pick a random port number.
+        You can then use serverthread.port to get the port number.
+
+        >>> port = 0
+        >>> serverthread = pydoc._start_server(my_url_handler, port)
+
+        Check that the server is really started.  If it is, open browser
+        and get first page.  Use serverthread.url as the starting page.
+
+        >>> if serverthread.serving:
+        ...    import webbrowser
+
+        The next two lines are commented out so a browser doesn't open if
+        doctest is run on this module.
+
+        #...    webbrowser.open(serverthread.url)
+        #True
+
+        Let the server do its thing. We just need to monitor its status.
+        Use time.sleep so the loop doesn't hog the CPU.
+
+        >>> starttime = time.monotonic()
+        >>> timeout = 1                    #seconds
+
+        This is a short timeout for testing purposes.
+
+        > while serverthread.serving:
+        .   time.sleep(.01)
+        .   if serverthread.serving and time.monotonic() - starttime > timeout:
+        .        serverthread.stop()
+        .        break
+
+        Print any errors that may have occurred.
+
+        >>> print(serverthread.error)
+        None
+   """
+    import http.server
+    import email.message
+    import select
+    import threading
+    import time
+
+    class DocHandler(http.server.BaseHTTPRequestHandler):
+
+        def do_GET(self):
+            """Process a request from an HTML browser.
+
+            The URL received is in self.path.
+            Get an HTML page from self.urlhandler and send it.
+            """
+            if self.path.endswith('.css'):
+                content_type = 'text/css'
+            else:
+                content_type = 'text/html'
+            self.send_response(200)
+            self.send_header(
+                'Content-Type', '%s; charset=UTF-8' % content_type)
+            self.end_headers()
+            try:
+                self.wfile.write(self.urlhandler(
+                    self.path, content_type).encode('utf-8'))
+            except ConnectionAbortedError:
+                # Needed to handle error when client closes the connection,
+                # for example when the client stops the load of the previously
+                # requested page. See spyder-ide/spyder#10755
+                pass
+
+        def log_message(self, *args):
+            # Don't log messages.
+            pass
+
+    class DocServer(http.server.HTTPServer):
+
+        def __init__(self, host, port, callback):
+            self.host = host
+            self.address = (self.host, port)
+            self.callback = callback
+            self.base.__init__(self, self.address, self.handler)
+            self.quit = False
+
+        def serve_until_quit(self):
+            while not self.quit:
+                rd, wr, ex = select.select([self.socket.fileno()], [], [], 1)
+                if rd:
+                    self.handle_request()
+            self.server_close()
+
+        def server_activate(self):
+            self.base.server_activate(self)
+            if self.callback:
+                self.callback(self)
+
+    class ServerThread(threading.Thread):
+
+        def __init__(self, urlhandler, host, port):
+            self.urlhandler = urlhandler
+            self.host = host
+            self.port = int(port)
+            threading.Thread.__init__(self)
+            self.serving = False
+            self.error = None
+
+        def run(self):
+            """Start the server."""
+            try:
+                DocServer.base = http.server.HTTPServer
+                DocServer.handler = DocHandler
+                DocHandler.MessageClass = email.message.Message
+                DocHandler.urlhandler = staticmethod(self.urlhandler)
+                docsvr = DocServer(self.host, self.port, self.ready)
+                self.docserver = docsvr
+                docsvr.serve_until_quit()
+            except Exception as e:
+                self.error = e
+
+        def ready(self, server):
+            self.serving = True
+            self.host = server.host
+            self.port = server.server_port
+            self.url = 'http://%s:%d/' % (self.host, self.port)
+
+        def stop(self):
+            """Stop the server and this thread nicely"""
+            self.docserver.quit = True
+            self.join()
+            # explicitly break a reference cycle: DocServer.callback
+            # has indirectly a reference to ServerThread.
+            self.docserver = None
+            self.serving = False
+            self.url = None
+
+    thread = ServerThread(urlhandler, hostname, port)
+    thread.start()
+    # Wait until thread.serving is True to make sure we are
+    # really up before returning.
+    while not thread.error and not thread.serving:
+        time.sleep(.01)
+    return thread
+
+
 class PydocServer(QThread):
     """Pydoc server"""
     server_started = Signal()
@@ -33,17 +217,11 @@ def __init__(self, port=7464):
         self.complete = False
 
     def run(self):
-        import pydoc
         if PY3:
             # Python 3
-            try:
-                self.callback(pydoc._start_server(pydoc._url_handler,
-                                                  port=self.port))
-            except TypeError:
-                # Python 3.7
-                self.callback(pydoc._start_server(pydoc._url_handler,
-                                                  hostname='127.0.0.1',
-                                                  port=self.port))
+            self.callback(_start_server(pydoc._url_handler,
+                                        hostname='127.0.0.1',
+                                        port=self.port))
         else:
             # Python 2
             pydoc.serve(self.port, self.callback, self.completer)
@@ -58,11 +236,11 @@ def completer(self):
     def quit_server(self):
         if PY3:
             # Python 3
-            if self.is_server_running() and self.server.serving:
+            if self.isRunning() and self.server.serving:
                 self.server.stop()
         else:
             # Python 2
-            if self.is_server_running():
+            if self.isRunning():
                 self.server.quit = 1
 
 
@@ -72,8 +250,9 @@ class PydocBrowser(WebBrowser):
     """
     DEFAULT_PORT = 30128
 
-    def __init__(self, parent, options_button=None):
-        WebBrowser.__init__(self, parent, options_button=options_button)
+    def __init__(self, parent, options_button=None, handle_links=False):
+        WebBrowser.__init__(self, parent, options_button=options_button,
+                            handle_links=handle_links)
         self.server = None
         self.port = None
 
@@ -123,14 +302,26 @@ def reload(self):
         self.start_server()
         WebBrowser.reload(self)
 
+    def load_finished(self, ok):
+        """Handle load finished."""
+        pass
+
     def text_to_url(self, text):
         """Convert text address into QUrl object"""
+        if text != 'about:blank':
+            text += '.html'
         if text.startswith('/'):
             text = text[1:]
-        return QUrl(self.home_url.toString()+text+'.html')
+
+        return QUrl(self.home_url.toString() + text)
 
     def url_to_text(self, url):
         """Convert QUrl object to displayed text in combo box"""
+        string_url = url.toString()
+        if 'about:blank' in string_url:
+            return 'about:blank'
+        elif 'get?key=' in string_url or 'search?key=' in string_url:
+            return url.toString().split('=')[-1]
         return osp.splitext(to_text_string(url.path()))[0][1:]
 
 
diff --git a/spyder/widgets/browser.py b/spyder/widgets/browser.py
index 84491a7b424..92b87959f45 100644
--- a/spyder/widgets/browser.py
+++ b/spyder/widgets/browser.py
@@ -47,12 +47,14 @@ def acceptNavigationRequest(self, url, navigation_type, isMainFrame):
         if navigation_type == QWebEnginePage.NavigationTypeLinkClicked:
             self.linkClicked.emit(url)
             return False
-        return True
+
+        return super(WebPage, self).acceptNavigationRequest(
+            url, navigation_type, isMainFrame)
 
 
 class WebView(QWebEngineView):
     """Web view"""
-    def __init__(self, parent):
+    def __init__(self, parent, handle_links=True):
         QWebEngineView.__init__(self, parent)
         self.zoom_factor = 1.
         self.zoom_out_action = create_action(self, _("Zoom out"),
@@ -62,7 +64,10 @@ def __init__(self, parent):
                                             icon=ima.icon('zoom_in'),
                                             triggered=self.zoom_in)
         if WEBENGINE:
-            web_page = WebPage(self)
+            if handle_links:
+                web_page = WebPage(self)
+            else:
+                web_page = QWebEnginePage(self)
             self.setPage(web_page)
             self.source_text = ''
 
@@ -209,12 +214,12 @@ class WebBrowser(QWidget):
     """
     Web browser widget
     """
-    def __init__(self, parent=None, options_button=None):
+    def __init__(self, parent=None, options_button=None, handle_links=True):
         QWidget.__init__(self, parent)
 
         self.home_url = None
 
-        self.webview = WebView(self)
+        self.webview = WebView(self, handle_links=handle_links)
         self.webview.loadFinished.connect(self.load_finished)
         self.webview.titleChanged.connect(self.setWindowTitle)
         self.webview.urlChanged.connect(self.url_changed)


Code review comments:
diff_hunk: @@ -22,6 +23,189 @@
 from spyder.widgets.browser import WebBrowser
 
 
+# =============================================================================
+# Pydoc adjustments
+# =============================================================================
+# This is needed to prevent pydoc raise an ErrorDuringImport when
+# trying to import numpy.
+# See spyder-ide/spyder#10740
+DIRECT_PYDOC_IMPORT_MODULES = ['numpy', 'numpy.core']
+try:
+    from pydoc import safeimport
+
+    def spyder_safeimport(path, forceload=0, cache={}):
+        if path in DIRECT_PYDOC_IMPORT_MODULES:
+            forceload = 0
+        return safeimport(path, forceload=forceload, cache=cache)
+
+    pydoc.safeimport = spyder_safeimport
+except Exception:
+    pass
+
+
+# A reimplementation of pydoc._start_server to handle connection errors
+# for 'do_GET'.
+def _start_server(urlhandler, hostname, port):
+    """Start an HTTP server thread on a specific port.
+
+    Taken from PyDoc: https://github.com/python/cpython/blob/3.7/Lib/pydoc.py
+    Start an HTML/text server thread, so HTML or text documents can be
+    browsed dynamically and interactively with a Web browser.  Example use:
path: spyder/plugins/onlinehelp/widgets.py
text: @user1:
Please remove all contents of this docstring below this line. I think they are not necessary because they from the original `_start_server`, right?

@user1:
Also, let's rewrite this docstring like this

```python
"""
Start an HTTP server thread on a specific port.

This is a reimplementation of `pydoc._start_server` to handle connection
errors for 'do_GET'.

Taken from PyDoc: https://github.com/python/cpython/blob/3.7/Lib/pydoc.py
"""
```

diff_hunk: @@ -22,6 +23,189 @@
 from spyder.widgets.browser import WebBrowser
 
 
+# =============================================================================
+# Pydoc adjustments
+# =============================================================================
+# This is needed to prevent pydoc raise an ErrorDuringImport when
+# trying to import numpy.
+# See spyder-ide/spyder#10740
+DIRECT_PYDOC_IMPORT_MODULES = ['numpy', 'numpy.core']
+try:
+    from pydoc import safeimport
+
+    def spyder_safeimport(path, forceload=0, cache={}):
+        if path in DIRECT_PYDOC_IMPORT_MODULES:
+            forceload = 0
+        return safeimport(path, forceload=forceload, cache=cache)
+
+    pydoc.safeimport = spyder_safeimport
+except Exception:
+    pass
+
+
+# A reimplementation of pydoc._start_server to handle connection errors
+# for 'do_GET'.
path: spyder/plugins/onlinehelp/widgets.py
text: @user1:
Let's move this to be part of the docstring of this function below.

diff_hunk: @@ -22,6 +23,189 @@
 from spyder.widgets.browser import WebBrowser
 
 
+# =============================================================================
+# Pydoc adjustments
+# =============================================================================
+# This is needed to prevent pydoc raise an ErrorDuringImport when
+# trying to import numpy.
+# See spyder-ide/spyder#10740
+DIRECT_PYDOC_IMPORT_MODULES = ['numpy', 'numpy.core']
+try:
+    from pydoc import safeimport
+
+    def spyder_safeimport(path, forceload=0, cache={}):
+        if path in DIRECT_PYDOC_IMPORT_MODULES:
+            forceload = 0
+        return safeimport(path, forceload=forceload, cache=cache)
+
+    pydoc.safeimport = spyder_safeimport
+except Exception:
+    pass
+
+
+# A reimplementation of pydoc._start_server to handle connection errors
+# for 'do_GET'.
+def _start_server(urlhandler, hostname, port):
path: spyder/plugins/onlinehelp/widgets.py
text: @user1:
Let's move this function to another module in the same directory of this file called `pydoc_patch.py`.