Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression for mmistakes#2332 #2385

Merged
merged 2 commits into from
Jan 29, 2020
Merged

Regression for mmistakes#2332 #2385

merged 2 commits into from
Jan 29, 2020

Conversation

iBug
Copy link
Collaborator

@iBug iBug commented Jan 29, 2020

This is a bug fix.

Summary

On the merged commit of #2332 (abf2943), there's already

<a class="site-logo" href="{{ '/' | relative_url }}"><img src="{{ logo_path | relative_url }}" alt=""></a>

Emphasis:

<img src="{{ logo_path | relative_url }}" alt="">
                       ^^^^^^^^^^^^^^

so the extra relative_url is the culprit for the duplicated base path for masthead logo when site.baseurl isn't resolved to /.

This PR resolves that bug by removing the extra relative_url. On a side note, I wouldn't blame the original PR suggester because the initial {% if site.logo contains "://" %} is rather misleading per se.

Context

#2332 Comment: #2332 (comment)

cc @Geekdude

There's already a `relative_url` in place, shouldn't stack up another
@iBug iBug mentioned this pull request Jan 29, 2020
@mmistakes
Copy link
Owner

mmistakes commented Jan 29, 2020

Can you verify this works with externally hosted images? That is what the //: check is for. There are users who don't host the images on their site (not sure why you'd want to do that but they do...).

So we need to be able to do logo: http://cdn.whatever.com/my-external-logo.png and logo: /assets/images/my-logo.png

@iBug
Copy link
Collaborator Author

iBug commented Jan 29, 2020

@mmistakes I've verified locally with both Jekyll 3.8.6 and Jekyll 4.0. Here are the results:

baseurl: /test
{{ "abcd" | relative_url }} -> /test/abcd
{{ "https://google.com/abcd" | relative_url }} -> https://google.com/abcd

So your worry isn't practical, Jekyll has already taken care of it with the relative_url filter.

@mmistakes
Copy link
Owner

OK cool. relative_url didn't use to work that way which is why that extra check was needed. Guess if they fixed that all the other spots in the theme where there's an absolute URL check could be removed. Think it's mostly in image includes... hero, galleries maybe, etc.

@mmistakes mmistakes merged commit 1799ddd into mmistakes:master Jan 29, 2020
@iBug iBug deleted the fix-2332 branch January 29, 2020 17:48
@iBug
Copy link
Collaborator Author

iBug commented Jan 29, 2020

@mmistakes Running git blame over the Jekyll code base shows jekyll/jekyll@a66c478 (merge of jekyll/jekyll#6490) on November 5, 2017. The milestone suggests that the change was carried out with the release of Jekyll 3.7.0. So if you want to utilize this new feature, you might want to update this badge in README.md:

[![Jekyll](https://img.shields.io/badge/jekyll-%3E%3D%203.6-blue.svg)](https://jekyllrb.com/)

I'd be happy to help if you want to perform the adoption of this change in Jekyll (3.6 → 3.7).

iBug added a commit to iBug/minimal-mistakes that referenced this pull request Jan 29, 2020
Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)
mmistakes pushed a commit that referenced this pull request Mar 6, 2020
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: #2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
ghost pushed a commit to ioniodi/minimal-ionio that referenced this pull request Jun 4, 2020
* Add UI Text for Thai Language (#2111)

* Update `/docs` text strings

* Add reference to Thai UI text strings

* Fix table of contents active link styling

* Add `max-width` Sass variable (#2093)

* Update CHANGELOG and history

* Allow adding JavaScript files after the theme's (#2116)

Allow adding JavaScript files after those bundled in the theme

Close #2110

* Update CHANGELOG and history

* Improve search `input` semantics (#2123)

* Update search_form.html

Updates Issue #2122 by adding the correct input type and aria label.

* update search_form.html

should be area-placeholder. My mistake. Updates Issue #2122

* fix aria-placeholder

Updates issue #2123 with a typo found by @mmistakes for liquid syntax.

Close #2122

* Update CHANGELOG and history

* Release 4.16.0 💎

* Made Gumshoe contingent upon a TOC being present. This prevents a console error on pages without a TOC. (#2124)

* Update CHANGELOG and history

* Update jquery v3.3.1 to v3.4.0 (#2129)

* Update jquery v3.3.1 to v3.4.0
* Update `main.min.js`

Close #2127

* Update CHANGELOG and history

* Update layout compress from v3.0.2 to v3.1.0 (#2128)

* Update CHANGELOG and history

* Add link to MM remote theme starter

* Release 4.16.1 💎

* Revert back to jQuery v3.3.1

3.4.0 causes issues with older jQuery plugins that haven't been updated to support it.

* Release 4.16.2 💎

* Update Gumshoe to v5.1.1

Fixes #2140

* Update CHANGELOG and history

* Update jQuery to v3.4.1

ref #2137

* Update JavaScript listing

* Update CHANGELOG and history

* Reset positioning of `rel="permalink"` anchors

* Release 4.16.3 💎

* Fix permalink stacking order

* Update CHANGELOG and history

* add www to facebook.com link (#2160)

* Change permalink

* Fix pound symbol not displaying properly for post categories and tags

Fixes #2156

* Add edge case

* Update CHANGELOG and history

* Create FUNDING.yml

* Fix and complete Spanish localization (#2149)

* Update CHANGELOG and history

* update Font Awesome (#2150)

REF: https://fontawesome.com/changelog/latest

* Update CHANGELOG and history

* Fix and improve pt-BR localization. (#2162)

* Update CHANGELOG and history

* Arithmetic fix in _form.scss (#2169)

* Update CHANGELOG and history

* Release 4.16.4 💎

* Remove duplicate UI text file in `/docs`

* Add skip links

Close #2182

* Remove extra `{`

Fixes #2192

* Low hanging fruit (#2186)

* Update 05-configuration.md

- fix dead Twitter Dev docs url
- slight clarification re: Player Card approval
- fix Jekyll docs url

* Update 2013-01-10-markup-image-alignment.md

- fix the/re typo

* Update CHANGELOG and history

* Fix broken Flickr images

* remove unneeded  type="text/javascript" (#2190)

REF: 
- https://google.github.io/styleguide/htmlcssguide.html#type_Attributes
- https://codeguide.co/#html-style-script
- https://developers.google.com/analytics/devguides/collection/analyticsjs/

* Update CHANGELOG and history

* Fix missing fallback title for table of contents

* Remove unecessary console.log in `lunr-en.js` and `lunr-gr.js` JavaScript

Close #2193

* Use Font Awesome Kits to use the latest version of icons

Close #2184

* Complete missing parts of Korean locale. (#2209)

search_placeholder_text, results_found, and back_to_top

* Update CHANGELOG and history

* Display site subtitle in masthead (#2205)

* Add site subtitle
* Tabs vs spaces - the ancient conflict
* updates cfr PR #2205
* Perhaps also add the closing anchor
* Make sure we check for the right variables...
* Brown paper bag - subtitle, not description

* Use `span` instead of `div` element

* Document `site.subtitle`

* Update CHANGELOG and history

* Add site subtitle

* Fixes aria issues on search form (#2211)

* Fixes aria issues on search form
* Swapping aria-label for <label> tag in search
* Removing background gray caused by adding <form> tag to search
* Removing redundant space
* Making form not submit if key is enter

Close #2180

* Add localized text string

* Update CHANGELOG and history

* Updated localized text string for "Punjabi" and "Hindi" language (#2212)

* Update CHANGELOG and history

* Release 4.16.5 💎

* Allow Markdown in author bio (#2215)

* enable markdown in author bio

added the markdownify liquid filter

* Add markdown to bio in _config.yml

* add markdown to bio in /test/ _config.yml

* Change <p> to <div>

* Add note about allowed Markdown in `author.bio`

* Update CHANGELOG and history

* Add emphasis

* Fix overlapping links in post link type

Close #2222

* Fix default site.author in seo.html (#2230)

* Update CHANGELOG and history

* Complete Spanish localization (#2229)

* Update CHANGELOG and history

* Remove fullstop in some Staticman UI text (#2220)

In this UI text, the 2nd sentence is not finished.
comment_form_info : "Your email address will not be published. Required fields are marked"

* Update CHANGELOG and history

* Fix typos

* Harmonize `site.url` for Organization JSON-LD schema

* Fix `site.url` in Organization/Person JSON-LD schema

Close #1906

* Update history

* Relax Jekyll dependency to allow for version 4.0

* Ignore Jekyll cache

* Remove jemoji gem

* Bump dependencies

* Remove jemoji gem

* Update documentation

* Release 4.16.6 💎

* Translate into Catalan (#2237)

* Translate into Catalan
* Translate missing key
* Pluralize key

* Update CHANGELOG, history, and docs

* Update onchange development dependency in package.json

Close #2241

* Fix typo in config comment (#2243)

* Update CHANGELOG and history

* Fix http links to use https (#2244)

Every site should support https by now. Better to link the https sites right away. Both sites do support https at the time I checked.

* Update CHANGELOG and history

* Allow per-page override of words_per_minute (#2250)

Different languages usually have different read speeds.
This change is useful for sites with multi-lingual content

* Document `page.words_per_minute`

* Update CHANGELOG and history

* Show a permalink anchor when hovering over headings in main content (#2251)

* Implement heading permalinks, close #2246

Thanks to jekyll/jekyll for CSS.
Link anchor is visible when the mouse hovers over the title line.

* Build the updated _main.js

* Update CHANGELOG and history

* remove extraneous space from IE conditional statement (#2273)

* Update CHANGELOG and history

* Release 4.17.0 (#2275)

* Release 4.17.0 💎

* Update CHANGELOG and history

* Update the info about what js scrips you are using (#2276)

Added Smooth Scroll and Gumshoe info

* Update CHANGELOG and history

* Fix and test case (#2285)

* Update CHANGELOG and history

* Update Chinese (Simplified) translation (#2286)

* Update CHANGELOG and history

* Release 4.17.1 💎 (#2288)

* Flexbox sticky footer (#2289)

* Remove sticky footer JavaScript

* Use flexbox to force footer to the bottom of every page

* Fix flexbox in Internet Explorer

* Update CHANGELOG and history

Close #2281

* Added another step (#2294)

The step is required to get the portfolio collection to work.

* Update CHANGELOG and history

* Release 4.17.2 💎 (#2296)

* Update CHANGELOG and history

Fix #2310

* Fix typo in _config.yml (#2319)

* Fix typo in comments in _config.yml

* Update CHANGELOG and history

* Support page header (page hero) in archive-taxonomy layouts (#2320)

* Support page header in archive-taxonomy layout

* Update CHANGELOG and history

* Add social icon color for Keybase (#2302)

* Update CHANGELOG and history

* Add missing dutch ui-text fields (#2321)

* Update CHANGELOG and history

* Fixed page.excerpt to seo_description (#2326)

* Update CHANGELOG and history

* Make feature row live well with TOC, fix #2327 (#2329)

* Update CHANGELOG and history

* Add notices with code blocks for testing

* Set background-color = inherit for <pre code> in notices (#2328)

* Update CHANGELOG and history

* Fix schema.org dates to ISO-8601 (#2339)

* Fix schema.org dates to ISO-8601

* Revert date_to_xmlschema in <time> tag

* Update masthead.html (#2332)

* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <[email protected]>

* Update CHANGELOG and history

* Fix staticman v2/v3 comments (#2351)

According to the most recent docs, `branch` and `endpoint` should be inside `site.comments.staticman` config.

* Update CHANGELOG and history

* Update toc.html from allejo/jekyll-toc:master (#2355)

* Update CHANGELOG and history

* Use %-d instead of %d (#2359)

* Update CHANGELOG and history

* 🔧 Exclude `package-lock.json` (#2364)

* Update CHANGELOG and history

* Allow override of page excerpt in heading (via tagline) (#2307)

Allow the use of `page.tagline` to override `page.excerpt` in heading area

* Add example posts with header `tagline`

* Document header `tagline`

* Update CHANGELOG and history

* Update .gitignore (#2366)

* Update CHANGELOG and history

* Update README and LICENSE (#2367)

* Update README and LICENSE

* README: Replace RawGit with jsDelivr

* Update CHANGELOG and history

* Fix wrong newline concatenation in SEO description, resolves #2354 (#2368)

Close #2354

* Add post to test multiline excerpt

* Update CHANGELOG and history

* Adjust comments to be compatible with compress_html (#2373)

* Update CHANGELOG and history

* Add links to high resolution skin screenshots

Resolves #2363

* Allow using theme without pagination (#2378)

* Update CHANGELOG and history

* Release 4.18.0 💎

* Fix compatibility issue with paginate V2 introduced by mmistakes/minimal-mistakes#2378 (#2381)

* Release 4.18.1 💎

* Regression for mmistakes#2332 (#2385)

* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history

* Bump Jekyll gem dependency to `v3.7`

* Disable box-shadow for radio and checkbox (#2398)

* Update CHANGELOG and history

* fix: Do not redirect user away from site after they submit a comment (#2402)

* Update CHANGELOG and history

* Fix rake vulnerability in .gemspec file

* Add Irish (Gaeilge) locale to ui-text.yml (#2422)

* Update CHANGELOG, history, and documentation

* Hide hidden posts from listings (#2345)

* Update CHANGELOG and history

* Improve accessiblity of default skin by increasing color contrast

* Update CHANGELOG and history

* Improve headline hierarchy

Add headline specific Sass variables:

```scss
$h-size-1: 1.563em !default; // ~25.008px
$h-size-2: 1.25em !default; // ~20px
$h-size-3: 1.125em !default; // ~18px
$h-size-4: 1.0625em !default; // ~17px
$h-size-5: 1.03125em !default; // ~16.5px
$h-size-6: 1em !default; // ~16px
```

Fixes #2423

* Use relative_url and absolute_url where possible (#2387)

* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes/minimal-mistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class

* Update CHANGELOG and history

* Fix minor deprecation issue (#2425)

with bundler show vs now info. Just to avoid warning for users

* Update CHANGELOG and history

* Fix typo in Sass variable map

* fixed typo in  page_hero (#2428)

* Lock demo site at version 4.18.1

ref: mmistakes/minimal-mistakes#2429

* Remove deprecated Staticman v1 config

Close #2386

* Update CHANGELOG and history

* Remove delay on menu close

* Add "click" overlay to close masthead and follow button menus when open

Fixes #1168

* Update CHANGELOG and history

* Remove unused variables

* Release 4.19.0 💎

* Force rebuild of site

* Update smooth-scroll.js to `v16.1.2`

Close #2430

* Fix author profile links `z-index` order on small screens

Close #2440

* Bump dependencies

* Update CHANGELOG and history

* Use first_page_path from Paginate V2 if available (#2431)

* Update CHANGELOG and history

* update links to HTTPS and remove Google+ from 05-configuration.md (#2432)

* Update CHANGELOG and history

* docs: Add dracula base16 syntax highlighting (#2438)

Updates stylesheets doc by adding a section for [dracula theme](https://draculatheme.com/).

* Update CHANGELOG and history

* Release 4.19.1 💎

* Bump theme version

* Clarify lunr only searches documents in collections (#2450)

* Update CHANGELOG and history

* Finnish translations (#2455)

* Finnish translations

* Small fix to finnish translation

* Update CHANGELOG and history

* Add mention of Finnish

* Update ui-text.yml for Vietnamese translation (#2459)

* Add guide on applying Front Matter defaults to jekyll-archives pages (#2466)

* Add guide on applying Front Matter defaults to jekyll-archives pages

Sources:
- mmistakes/minimal-mistakes#2465 (comment)
- iBug/iBug-source@8685c1e

* Update CHANGELOG and history

* Update CHANGELOG and history

* Use css padding for author urls to fix underline hover state (#2472)

* Use scss padding for sidebar author urls

* rename .text to .label

* Update CHANGELOG and history

* Fix #2478 "Follow menu falls under post links" (#2479)

* Fix #2478 "Follow menu falls under post links"

* Do it the other way

* Hide index page from page-archive, resolves #2481 (#2482)

* Update README.md

* Update 01-quick-start-guide.md

* Update Vietnamese translation (#2486)

* Update Vietnamese translation

* Update Vietnamese translation

* Update CHANGELOG and history

* Fix typo in configuration docs (#2497)

`https://github.io.mmistakes` should be `https://mmistakes.github.io`

* Update CHANGELOG and history

* Update Burmese translation (#2500)

* Update CHANGELOG and history

* Add Myanmar (Burmese)

* Update video (#2512)

* Update video

support bilibili video

* Update video

drop the duplicated <div> wrapper

* Update video

update video

* Add bilibili provider

* Update CHANGELOG and history

* Fix typo

* Add bilibili provider

* Wrap email in quotes

Close #2498

* Release 4.19.2 💎

* Bump version to 4.19.2

* Fix typo

* Ignore heading in TOC

Co-authored-by: Samiti <[email protected]>
Co-authored-by: Michael Rose <[email protected]>
Co-authored-by: Luis Puerto <[email protected]>
Co-authored-by: Michael Rose <[email protected]>
Co-authored-by: Denver Prophit Jr <[email protected]>
Co-authored-by: Jeckari <[email protected]>
Co-authored-by: Alex Malaszkiewicz <[email protected]>
Co-authored-by: Christian Oliff <[email protected]>
Co-authored-by: Juan Diego <[email protected]>
Co-authored-by: Vítor Bernardes <[email protected]>
Co-authored-by: Emanuele Barsanti <[email protected]>
Co-authored-by: Noah Pivnick <[email protected]>
Co-authored-by: Potados <[email protected]>
Co-authored-by: Jan De Luyck <[email protected]>
Co-authored-by: Jim Drury <[email protected]>
Co-authored-by: Kulbhushan Chand <[email protected]>
Co-authored-by: Ravi <[email protected]>
Co-authored-by: Jason Thai <[email protected]>
Co-authored-by: Vincent Tam <[email protected]>
Co-authored-by: Marc Riera <[email protected]>
Co-authored-by: Rico <[email protected]>
Co-authored-by: iBug ♦ <[email protected]>
Co-authored-by: Baptiste M <[email protected]>
Co-authored-by: Dan Wilde <[email protected]>
Co-authored-by: Wouter Janson <[email protected]>
Co-authored-by: Wouter Overmeire <[email protected]>
Co-authored-by: Lars Olesen <[email protected]>
Co-authored-by: Eloi Perdereau <[email protected]>
Co-authored-by: dmitrypv <[email protected]>
Co-authored-by: Akhyar Amarullah <[email protected]>
Co-authored-by: Rodrigo Graça <[email protected]>
Co-authored-by: Johannes Ganzenmüller <[email protected]>
Co-authored-by: Mohamed Akram <[email protected]>
Co-authored-by: Daniel Schroeder <[email protected]>
Co-authored-by: Ivan Storck <[email protected]>
Co-authored-by: chrismelman <[email protected]>
Co-authored-by: Christian Oliff <[email protected]>
Co-authored-by: Luiz Lopes <[email protected]>
Co-authored-by: A. Ishikawa <[email protected]>
Co-authored-by: Tatu Wikman <[email protected]>
Co-authored-by: Quan <[email protected]>
Co-authored-by: Ali Farooq <[email protected]>
Co-authored-by: Tobias V. Langhoff <[email protected]>
Co-authored-by: Hein Htet Aung <[email protected]>
Co-authored-by: sunete <[email protected]>
jesuswasrasta pushed a commit to jesuswasrasta/jesuswasrasta.github.io that referenced this pull request Jul 8, 2020
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
jesuswasrasta pushed a commit to jesuswasrasta/jesuswasrasta.github.io that referenced this pull request Jul 8, 2020
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
jrcasey pushed a commit to jrcasey/jrcasey.github.io that referenced this pull request Aug 18, 2020
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes/minimal-mistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
npflueger pushed a commit to npflueger/npflueger.github.io that referenced this pull request Dec 30, 2020
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes/minimal-mistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
mzaffran pushed a commit to mzaffran/mzaffran.github.io that referenced this pull request Jan 4, 2021
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
kaitokikuchi pushed a commit to kaitokikuchi/kaitokikuchi.github.io that referenced this pull request Sep 4, 2023
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
chukycheese pushed a commit to chukycheese/chukycheese.github.io that referenced this pull request Sep 18, 2023
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
chukycheese pushed a commit to chukycheese/chukycheese.github.io that referenced this pull request Sep 18, 2023
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants