Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Brave doesn't remember last position and size if homepage is set #3786

Closed
wants to merge 1 commit into from
Closed

Brave doesn't remember last position and size if homepage is set #3786

wants to merge 1 commit into from

Conversation

Sh1d0w
Copy link

@Sh1d0w Sh1d0w commented Sep 7, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Brave doesn't remember last position and size if you set it to start with homepage

When closed Brave does not retain last window position and size as it should be when re-launched.

This happens only if in Settings -> General you set the value for Brave starts with to My homepage. If you select Session and tabs from last time it correctly remembers the position and the size of the window.

Fixes ##3754

Auditors
@bbondy

Test Plan:

  1. In settings -> general set Brave starts with to My homepage
  2. Launch Brave, move the window around and close
  3. Re-launch Brave and compare the last window position

…with homepage

When closed Brave does not retain last window position and size as it should be when re-launched.

This happens only if in Settings -> General you set the value for Brave starts with to My homepage. If you select Session and tabs from last time it correctly remembers the position and the size of the window.

Fixes ##3754

Auditors
@bbondy

Test Plan:
1. In settings -> general set Brave starts with to My homepage
2. Launch Brave, move the window around and close
3. Re-launch Brave and compare the last window position
@Sh1d0w Sh1d0w added the bug label Sep 7, 2016
@Sh1d0w Sh1d0w added this to the 0.12.1dev milestone Sep 7, 2016
@@ -59,7 +59,14 @@ module.exports.saveAppState = (payload, isShutdown) => {
wndPayload.tabs = wndPayload.tabs.filter((tab) => !tab.isPrivate)
})
} else {
delete payload.perWindowState
// Reset the window state except for the ui data
payload.perWindowState.forEach((wndPayload) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think there could be a case somewhere where payload.perWindowState is empty.

Copy link
Author

Choose a reason for hiding this comment

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

Since it will be an empty array I don't see any problem if it is not unset, because on the next initialization it will be set to empty array again. Let me know if you want me to add check tho

Copy link
Member

Choose a reason for hiding this comment

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

specifically [] is not falsy so maybe the check in the block above is superfluous if (payload.perWindowState && savePerWindowState) { but it makes me wonder if it can be undefined and not empty sometimes. It could be an old check.

@bbondy bbondy removed this from the 0.12.1dev milestone Sep 9, 2016
@bbondy
Copy link
Member

bbondy commented Sep 12, 2016

I think you have to change the default window size and position in appstate and not window state.

@bbondy bbondy closed this Sep 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants