Skip to content

Force HTTPS of google-analytics.swig. Again.#881

Merged
ivan-nginx merged 1 commit intotheme-next:masterfrom
hsiangfeng:patch-1
May 20, 2019
Merged

Force HTTPS of google-analytics.swig. Again.#881
ivan-nginx merged 1 commit intotheme-next:masterfrom
hsiangfeng:patch-1

Conversation

@hsiangfeng
Copy link
Member

@hsiangfeng hsiangfeng commented May 20, 2019 •
edited
Loading

No "https" will result in an error。

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes was maked (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs in NexT website have been added / updated (for new features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Issue resolved: N/A

What is the new behavior?

  • Screenshots with this changes: N/A
  • Link to demo site with this changes: N/A

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

  • Yes.
  • No.

@hsiangfeng
Update google-analytics.swig
bd61ec3 
No "https" will result in an error。
@stevenjoezhang stevenjoezhang added this to the 7.2.0 milestone May 20, 2019
@1v9
Copy link
Member

1v9 commented May 20, 2019

What's the error?

@ivan-nginx
Copy link
Member

ivan-nginx commented May 20, 2019 •
edited
Loading

I think there is no error because auto-protocol must be used. But need to test it: can GA use link without schema or not?

@stevenjoezhang u approve it, but u test it? Test old method? Test new?

@ivan-nginx ivan-nginx removed this from the 7.2.0 milestone May 20, 2019
@ivan-nginx ivan-nginx added this to the 7.2.0 milestone May 20, 2019
@hsiangfeng
Copy link
Member Author

hsiangfeng commented May 20, 2019 •
edited
Loading

The organic rate will not be able to obtain GA.

It is recommended to use "https" directly because it sometimes becomes "http"

Can see here

I have encountered。

@ivan-nginx
Copy link
Member

ivan-nginx commented May 20, 2019 •
edited
Loading

What's this «it sometimes becomes "http"»? If site proto http, http will be used; if https, will be used https. All another similar 3rd-party services working like that. I think if protocol of website will be http, GA with your changes (https) will not work.

Can u provide more info about that GA usage recommendations?

Alright, I see now.

@ivan-nginx ivan-nginx removed this from the 7.2.0 milestone May 20, 2019
ivan-nginx
ivan-nginx approved these changes May 20, 2019
View reviewed changes
Copy link
Member

@ivan-nginx ivan-nginx left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that fix-unfix was changed many times: #580

But in accordance with that comment and check both links directly I think more reasonable will be usage with force https proto.

@ivan-nginx ivan-nginx added this to the 7.2.0 milestone May 20, 2019
@ivan-nginx ivan-nginx changed the title Update google-analytics.swig Force HTTPS of google-analytics.swig. Again. May 20, 2019
@ivan-nginx ivan-nginx merged commit b719ee7 into theme-next:master May 20, 2019
@hsiangfeng hsiangfeng deleted the patch-1 branch May 20, 2019 16:06
@Raincal
Copy link
Member

Raincal commented May 20, 2019 •
edited by unfurl-links bot
Loading

https://google.github.io/styleguide/htmlcssguide.html#Protocol

@ivan-nginx
Copy link
Member

ivan-nginx commented May 20, 2019

Seems fonts need to be fixed too:

hexo-theme-next/layout/_partials/head/external-fonts.swig

Line 47 in d78a400

{% set font_host = font_config.host | default('//fonts.googleapis.com') %}

jiangtj pushed a commit to jiangtj/hexo-theme-next that referenced this pull request May 21, 2019
@hsiangfeng @jiangtj
Force HTTPS of google-analytics.swig. Again. (theme-next#881)
85b6bc7 
No "https" will result in an error
invkrh added a commit to invkrh/hexo-theme-next that referenced this pull request Jun 16, 2019
@invkrh
Merge tag 'v7.1.2'
8f792a9 
* tag 'v7.1.2':
  Release v7.1.2
  Quotes in Release Drafter for around possible bugs
  Add Icons in Release Drafter
  Expressions and translations in CONTRIBUTING.md (theme-next#885)
  Fix Release Drafter template
  `External Change` label added
  Change the internal path of quicklink (theme-next#883)
  Improve Docs section in PR template
  Force HTTPS of `google-analytics.swig`. Again. (theme-next#881)
  Update Facebook Graph API to v3.3 (theme-next#876)
  Fix a few typo errors in docs (theme-next#864)
  Changed Telegram group link to Chinese
  Fix Github gist bug (theme-next#861)
  Update symbols_count_time (theme-next#862)
  Release v7.1.1
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
@hsiangfeng @ivan-nginx
Force HTTPS of google-analytics.swig. Again. (theme-next#881)
ba24c6b 
No "https" will result in an error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivan-nginx ivan-nginx ivan-nginx approved these changes

@maple3142 maple3142 Awaiting requested review from maple3142

@sli1989 sli1989 Awaiting requested review from sli1989

+1 more reviewer

@stevenjoezhang stevenjoezhang stevenjoezhang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🐞 Bug Fix

Projects

None yet

Milestone

v7.1.2

Development

Successfully merging this pull request may close these issues.

5 participants

@hsiangfeng @1v9 @ivan-nginx @Raincal @stevenjoezhang