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

Cookie uses dnn_IsMobile with term "dnn" #4044

Closed
4 tasks
thabaum opened this issue Sep 3, 2020 · 4 comments · Fixed by #4064
Closed
4 tasks

Cookie uses dnn_IsMobile with term "dnn" #4044

thabaum opened this issue Sep 3, 2020 · 4 comments · Fixed by #4064

Comments

@thabaum
Copy link
Contributor

thabaum commented Sep 3, 2020

Description of bug

Not really a bug but a preference, I would like to see less references to the platform as possible and I believe changing the cookie used for dnn_IsMobile could be changed to something else less revealing

Steps to reproduce

List the precise steps to reproduce the bug:

  1. Go to a dnn site
  2. Click on security lock to view cookies used in browser
  3. Scroll to dnn_IsMobile

Current behavior

cookie uses dnn_IsMobile

Expected behavior

something more generic that is less revealing used instead of terms like "dnn" in cookie.

Screenshots

If applicable, provide screenshots to help explain the bug.
image

Additional context

I will create a PR using app_IsMobile for a more generic cookie name

Affected version

  • [x ] 10.00.00 alpha build
  • [x ] 09.07.01 release candidate
  • [x ] 09.07.00 latest supported release

Affected browser

  • [x ] Chrome
  • Firefox
  • Safari
  • Internet Explorer 11
  • Microsoft Edge (Classic)
  • [x ] Microsoft Edge Chromium
@bdukes
Copy link
Contributor

bdukes commented Sep 3, 2020

Folks do have configurations and documentation making use of that name, so just changing the name would be a breaking change for some people. Making the name configurable would not be a breaking change, and it much more likely to get accepted. Hope it helps!

@thabaum
Copy link
Contributor Author

thabaum commented Sep 3, 2020

I am not into adding work potentially to any folks workloads. I would like to create the solution as you described which was my first go to fix, but thought maybe the simple fix would be good to go...

Looking into a web.config solution as proposed here would it be something like adding the following:

<appSettings>
<add key="MobileViewSiteCookieName" value="dnn_IsMobile"/>
<add key="DisableMobileViewCookieName" value="dnn_NoMobile"/>
</appSettings>

Should the Settings be in <appSettings> or in <dotnetnuke> web.config settings? Or should it be in it's own place?

Then in FriendlyUrlController.cs file edit to select these keys. ( DNN Platform/Library/Entities/Urls/FriendlyUrlController.cs line 34 about )

After adding using System.Configuration to the top of the file I could change the lines of code as shown and things should work am I correct?

private readonly string MobileViewSiteCookieName = ConfigurationManager.AppSettings[name: "MobileViewSiteCookieName"];
private readonly string DisableMobileViewCookieName = ConfigurationManager.AppSettings[name: "DisableMobileViewSiteCookieName"];

I got this far with VS not yelling at me except to make the first letter lowercase in the readonly string which I had to change from const... which then of coarse I would need to change the names changed throughout the class where they are referenced.

I know this isnt 100% correct I am just putting down some ideas here... bare with me sorry :(

If so how do I go about making sure the new app setting is added to everyones web configs?

Am I heading in the right direction here?

Thanks again for the support assisting me with this issue!

@bdukes
Copy link
Contributor

bdukes commented Sep 4, 2020

One thing you should probably do is default to the old value, and then you don't need to add it to existing web.config files. It could look like this:

private readonly string MobileViewSiteCookieName = ConfigurationManager.AppSettings[name: "MobileViewSiteCookieName"] ?? "dnn_IsMobile";
private readonly string DisableMobileViewCookieName = ConfigurationManager.AppSettings[name: "DisableMobileViewSiteCookieName"] ?? "dnn_NoMobile";

You can also see 09.08.00.config from #3969 for an example of adding new elements to the web.config

@thabaum
Copy link
Contributor Author

thabaum commented Sep 5, 2020

I will try to have this done up today, I was hoping to work on the PR last night...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants