Fix first_image not resolving correctly in subdirectories (#61)

ItayZiv · TheTripleV · web-flow · commit ada5be9f2942 · 2022-03-08T14:16:23.000-06:00
Co-authored-by: TheTripleV &lt;vasistavovveti@gmail.com&gt;
diff --git a/sphinxext/opengraph/__init__.py b/sphinxext/opengraph/__init__.py
@@ -130,12 +130,19 @@ def get_tags(
 
     if image_url:
         # temporarily disable relative image paths with field lists
-        if image_url and "og:image" not in fields:
+        if "og:image" not in fields:
             image_url_parsed = urlparse(image_url)
             if not image_url_parsed.scheme:
-                # Relative image path detected. Make absolute.
-                image_url = urljoin(config["ogp_site_url"], image_url_parsed.path)
-        tags["og:image"] = image_url
+                # Relative image path detected, relative to the source. Make absolute.
+                if config["ogp_image"]:
+                    # ogp_image is defined as being relative to the site root.
+                    # This workaround is to keep that functionality from breaking.
+                    root = config["ogp_site_url"]
+                else:
+                    root = page_url
+
+                image_url = urljoin(root, image_url_parsed.path)
+            tags["og:image"] = image_url
 
         # Add image alt text (either provided by config or from site_name)
         if isinstance(ogp_image_alt, str):
diff --git a/tests/roots/test-arbitrary-tags/conf.py b/tests/roots/test-arbitrary-tags/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-arbitrary-tags/index.rst b/tests/roots/test-arbitrary-tags/index.rst
@@ -1,4 +1,4 @@
-:og:video: http://example.org/video.mp4
+:og:video: http://example.org/en/latest/video.mp4
 :og:video:type: video/mp4
 
 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse at lorem ornare, fringilla massa nec, venenatis mi. Donec erat sapien, tincidunt nec rhoncus nec, scelerisque id diam. Orci varius natoque penatibus et magnis dis parturient mauris.
diff --git a/tests/roots/test-custom-tags/conf.py b/tests/roots/test-custom-tags/conf.py
@@ -5,7 +5,7 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 
 ogp_custom_meta_tags = [
     '<meta property="og:ignore_canonical" content="true" />',
diff --git a/tests/roots/test-description-length/conf.py b/tests/roots/test-description-length/conf.py
@@ -5,5 +5,5 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_description_length = 50
diff --git a/tests/roots/test-double-spacing/conf.py b/tests/roots/test-double-spacing/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-first-image-no-image/conf.py b/tests/roots/test-first-image-no-image/conf.py
@@ -6,7 +6,7 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
-ogp_image = "http://example.org/image33.png"
+ogp_site_url = "http://example.org/en/latest/"
+ogp_image = "http://example.org/en/latest/image33.png"
 ogp_image_alt = "TEST"
 ogp_use_first_image = True
diff --git a/tests/roots/test-first-image/conf.py b/tests/roots/test-first-image/conf.py
@@ -6,7 +6,7 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
-ogp_image = "http://example.org/image33.png"
+ogp_site_url = "http://example.org/en/latest/"
+ogp_image = "http://example.org/en/latest/image33.png"
 ogp_image_alt = "TEST"
 ogp_use_first_image = True
diff --git a/tests/roots/test-first-image/index.rst b/tests/roots/test-first-image/index.rst
@@ -1,2 +1,2 @@
-.. image:: http://example.org/image2.png
+.. image:: http://example.org/en/latest/image2.png
    :alt: Test image alt text
diff --git a/tests/roots/test-image-rel-paths/conf.py b/tests/roots/test-image-rel-paths/conf.py
@@ -6,5 +6,5 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_use_first_image = True
diff --git a/tests/roots/test-image/conf.py b/tests/roots/test-image/conf.py
@@ -6,5 +6,5 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
-ogp_image = "http://example.org/image.png"
+ogp_site_url = "http://example.org/en/latest/"
+ogp_image = "http://example.org/en/latest/image.png"
diff --git a/tests/roots/test-list/conf.py b/tests/roots/test-list/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-local-image/_static/sample.jpg b/tests/roots/test-local-image/_static/sample.jpg
diff --git a/tests/roots/test-local-image/conf.py b/tests/roots/test-local-image/conf.py
@@ -0,0 +1,10 @@
+extensions = ["sphinxext.opengraph"]
+
+master_doc = "index"
+exclude_patterns = ["_build"]
+
+html_theme = "basic"
+
+ogp_site_name = "Example's Docs!"
+ogp_site_url = "http://example.org/en/latest/"
+ogp_image = "_static/sample.jpg"
diff --git a/tests/roots/test-local-image/index.rst b/tests/roots/test-local-image/index.rst
diff --git a/tests/roots/test-nested-lists/conf.py b/tests/roots/test-nested-lists/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-overrides-complex/conf.py b/tests/roots/test-overrides-complex/conf.py
@@ -6,5 +6,5 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_image_alt = "Example Alt Text"
diff --git a/tests/roots/test-overrides-simple/conf.py b/tests/roots/test-overrides-simple/conf.py
@@ -6,6 +6,6 @@
 html_theme = "basic"
 
 ogp_site_name = "Example's Docs!"
-ogp_site_url = "http://example.org/"
-ogp_image = "http://example.org/image.png"
+ogp_site_url = "http://example.org/en/latest/"
+ogp_image = "http://example.org/en/latest/image.png"
 ogp_type = "book"
diff --git a/tests/roots/test-overrides-simple/index.rst b/tests/roots/test-overrides-simple/index.rst
@@ -1,7 +1,7 @@
 :og:description: Overridden description
 :og:title: Overridden Title
 :og:type: article
-:og:image: http://example.org/overridden-image.png
+:og:image: http://example.org/en/latest/overridden-image.png
 
 Lorem Ipsum
 ===========
diff --git a/tests/roots/test-quotation-marks/conf.py b/tests/roots/test-quotation-marks/conf.py
@@ -7,4 +7,4 @@
 
 smartquotes = False
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-simple/conf.py b/tests/roots/test-simple/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-sitename/conf.py b/tests/roots/test-sitename/conf.py
@@ -5,5 +5,5 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_site_name = "Example's Docs!"
diff --git a/tests/roots/test-skip-admonitions/conf.py b/tests/roots/test-skip-admonitions/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-skip-code-block/conf.py b/tests/roots/test-skip-code-block/conf.py
@@ -5,5 +5,5 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_description_length = 100
diff --git a/tests/roots/test-skip-comments/conf.py b/tests/roots/test-skip-comments/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-skip-raw/conf.py b/tests/roots/test-skip-raw/conf.py
@@ -5,5 +5,5 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_description_length = 100
diff --git a/tests/roots/test-skip-title/conf.py b/tests/roots/test-skip-title/conf.py
@@ -5,4 +5,4 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
diff --git a/tests/roots/test-type/conf.py b/tests/roots/test-type/conf.py
@@ -5,5 +5,5 @@
 
 html_theme = "basic"
 
-ogp_site_url = "http://example.org/"
+ogp_site_url = "http://example.org/en/latest/"
 ogp_type = "article"
diff --git a/tests/test_options.py b/tests/test_options.py
@@ -29,28 +29,30 @@ def test_simple(og_meta_tags):
 @pytest.mark.sphinx("html", testroot="simple")
 def test_site_url(og_meta_tags):
     # Uses the same directory as simple, because it already contains url for a minimal config
-    assert get_tag_content(og_meta_tags, "url") == "http://example.org/index.html"
+    assert (
+        get_tag_content(og_meta_tags, "url")
+        == "http://example.org/en/latest/index.html"
+    )
 
 
 @pytest.mark.sphinx("dirhtml", testroot="simple")
 def test_dirhtml_url(og_meta_tags):
-    assert get_tag_content(og_meta_tags, "url") == "http://example.org/index/"
+    assert get_tag_content(og_meta_tags, "url") == "http://example.org/en/latest/index/"
 
 
 @pytest.mark.sphinx("html", testroot="image")
 def test_image(og_meta_tags):
-    assert get_tag_content(og_meta_tags, "image") == "http://example.org/image.png"
-
-
-@pytest.mark.sphinx("html", testroot="image-rel-paths")
-def test_image_rel_paths(og_meta_tags, og_meta_tags_sub):
     assert (
         get_tag_content(og_meta_tags, "image")
-        == "http://example.org/_images/sample.jpg"
+        == "http://example.org/en/latest/image.png"
     )
+
+
+@pytest.mark.sphinx("html", testroot="local-image")
+def test_local_image(og_meta_tags):
     assert (
-        get_tag_content(og_meta_tags_sub, "image")
-        == "http://example.org/_images/sample.jpg"
+        get_tag_content(og_meta_tags, "image")
+        == "http://example.org/en/latest/_static/sample.jpg"
     )
 
 
@@ -76,16 +78,34 @@ def test_site_name(og_meta_tags):
 
 @pytest.mark.sphinx("html", testroot="first-image")
 def test_first_image(og_meta_tags):
-    assert get_tag_content(og_meta_tags, "image") == "http://example.org/image2.png"
+    assert (
+        get_tag_content(og_meta_tags, "image")
+        == "http://example.org/en/latest/image2.png"
+    )
     assert get_tag_content(og_meta_tags, "image:alt") == "Test image alt text"
 
 
 @pytest.mark.sphinx("html", testroot="first-image-no-image")
 def test_first_image_no_image(og_meta_tags):
-    assert get_tag_content(og_meta_tags, "image") == "http://example.org/image33.png"
+    assert (
+        get_tag_content(og_meta_tags, "image")
+        == "http://example.org/en/latest/image33.png"
+    )
     assert get_tag_content(og_meta_tags, "image:alt") == "TEST"
 
 
+@pytest.mark.sphinx("html", testroot="image-rel-paths")
+def test_image_rel_paths(og_meta_tags, og_meta_tags_sub):
+    assert (
+        get_tag_content(og_meta_tags, "image")
+        == "http://example.org/en/latest/_images/sample.jpg"
+    )
+    assert (
+        get_tag_content(og_meta_tags_sub, "image")
+        == "http://example.org/en/latest/_images/sample.jpg"
+    )
+
+
 @pytest.mark.sphinx("html", testroot="skip-admonitions")
 def test_skip_admonitions(og_meta_tags):
     assert get_tag_content(og_meta_tags, "description") == "This is text."
@@ -172,7 +192,7 @@ def test_overrides_simple(og_meta_tags):
     assert get_tag_content(og_meta_tags, "type") == "article"
     assert (
         get_tag_content(og_meta_tags, "image")
-        == "http://example.org/overridden-image.png"
+        == "http://example.org/en/latest/overridden-image.png"
     )
     # Make sure alt text still works even when overriding the image
     assert get_tag_content(og_meta_tags, "image:alt") == "Example's Docs!"
@@ -181,37 +201,43 @@ def test_overrides_simple(og_meta_tags):
 @pytest.mark.sphinx("html", testroot="overrides-complex")
 def test_overrides_complex(og_meta_tags):
     assert len(get_tag_content(og_meta_tags, "description")) == 10
-    assert get_tag_content(og_meta_tags, "image") == "http://example.org/img/sample.jpg"
+    assert (
+        get_tag_content(og_meta_tags, "image")
+        == "http://example.org/en/latest/img/sample.jpg"
+    )
     assert get_tag_content(og_meta_tags, "image:alt") == "Overridden Alt Text"
 
 
 @pytest.mark.sphinx("html", testroot="arbitrary-tags")
 def test_arbitrary_tags(og_meta_tags):
-    assert get_tag_content(og_meta_tags, "video") == "http://example.org/video.mp4"
+    assert (
+        get_tag_content(og_meta_tags, "video")
+        == "http://example.org/en/latest/video.mp4"
+    )
     assert get_tag_content(og_meta_tags, "video:type") == "video/mp4"
 
 
 # use same as simple, as configuration is identical to overriden
 @pytest.mark.sphinx("html", testroot="simple")
 def test_rtd_override(app: Sphinx, monkeypatch):
     monkeypatch.setenv("READTHEDOCS", "True")
-    app.config.html_baseurl = "https://failure.com"
+    app.config.html_baseurl = "https://failure.com/en/latest/"
 
     app.build()
     tags = conftest._og_meta_tags(app)
 
-    assert get_tag_content(tags, "url") == "http://example.org/index.html"
+    assert get_tag_content(tags, "url") == "http://example.org/en/latest/index.html"
 
 
 @pytest.mark.sphinx("html", testroot="rtd-default")
 def test_rtd_valid(app: Sphinx, monkeypatch):
     monkeypatch.setenv("READTHEDOCS", "True")
-    app.config.html_baseurl = "https://failure.com"
+    app.config.html_baseurl = "https://failure.com/en/latest/"
 
     app.build()
     tags = conftest._og_meta_tags(app)
 
-    assert get_tag_content(tags, "url") == "https://failure.com/index.html"
+    assert get_tag_content(tags, "url") == "https://failure.com/en/latest/index.html"
 
 
 # use rtd-default, as we are not changing configuration, but RTD variables

Original file line number	Diff line number	Diff line change
`@@ -5,4 +5,4 @@`
`5`	`5`
`6`	`6`	`html_theme = "basic"`
`7`	`7`
`8`		`-ogp_site_url = "http://example.org/"`
	`8`	`+ogp_site_url = "http://example.org/en/latest/"`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`		`-.. image:: http://example.org/image2.png`
	`1`	`+.. image:: http://example.org/en/latest/image2.png`
`2`	`2`	`:alt: Test image alt text`