Fixes window restore and fullscreen #10458

NejcZdovc · 2017-08-13T11:01:41Z

Please don't squash commits

Submitter Checklist:

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).
Tagged reviewers and labelled the pull request as needed.

Resolves #3558
Resolves #3754
Resolves #6602
Resolves #8600
Resolves #8925
Resolves #9371
Resolves #9709

Test Plan:

Reviewer Checklist:

Tests

Adequate test coverage exists to prevent regressions
Tests should be independent and work correctly when run individually or as a suite ref
New files have MPL2 license header

codecov-io · 2017-08-13T11:20:14Z

Codecov Report

Merging #10458 into master will decrease coverage by 0.36%.
The diff coverage is 11.59%.

@@            Coverage Diff             @@
##           master   #10458      +/-   ##
==========================================
- Coverage   54.54%   54.17%   -0.37%     
==========================================
  Files         245      246       +1     
  Lines       21156    21374     +218     
  Branches     3263     3316      +53     
==========================================
+ Hits        11539    11579      +40     
- Misses       9617     9795     +178

Flag	Coverage Δ
#unittest	`54.17% <11.59%> (-0.37%)`	⬇️

Impacted Files	Coverage Δ
js/actions/appActions.js	`12.94% <ø> (+0.09%)`	⬆️
js/constants/appConstants.js	`100% <ø> (ø)`	⬆️
js/constants/windowConstants.js	`100% <ø> (ø)`	⬆️
app/browser/reducers/tabsReducer.js	`49.36% <ø> (-0.8%)`	⬇️
js/stores/windowStore.js	`30.28% <0%> (+0.51%)`	⬆️
js/contextMenus.js	`17.92% <0%> (ø)`	⬆️
js/actions/windowActions.js	`5.71% <0%> (+0.25%)`	⬆️
js/state/frameStateUtil.js	`55.91% <100%> (ø)`	⬆️
app/sessionStore.js	`74.32% <12.5%> (-0.92%)`	⬇️
app/browser/windows.js	`18.43% <22.22%> (+0.6%)`	⬆️
... and 12 more

@bbondy

Pre-work for brave#10458 tests Auditors: @bbondy @bsclifton Test Plan: - covered by automated tests

@bbondy

Pre-work for brave#10458 tests Auditors: @bbondy @bsclifton Test Plan: - covered by automated tests

luixxiul · 2017-08-15T03:08:54Z

I'm wondering if this could be merged to 0.21 to do enough QA over releases among various environments

NejcZdovc · 2017-08-15T06:27:19Z

This is targeted for 0.20, so we should keep it here

NejcZdovc · 2017-08-15T13:12:59Z

This PR is complete, I am just writing some tests

bsclifton · 2017-08-23T06:49:17Z

I just did some manual testing- it seems all issues are resolved except for #3754

It always centers the window 🤔

bsclifton

Lots of GREAT cleanup in this PR! Awesome job 😄

Comment left about one issue which didn't appear to be fixed

NejcZdovc · 2017-08-25T08:11:04Z

@bsclifton #3754 should be fixed now as well. Can you please re-test?

Resolves brave#3754 Resolves brave#6602 Resolves brave#8600 Resolves brave#8925 Resolves brave#9709 Auditors: Test Plan:

NejcZdovc · 2017-08-25T09:57:10Z

Webdriver test for #3754 added

@NejcZdovc

This was causing a crash which breaks the browser window. I believe this issue is related to the issues we have where wrong index is picked after closing / tearing off tab Auditors: @NejcZdovc, @bbondy

bsclifton

I confirmed each issue is working as expected 😄 Great job on this

FYI - I ran into an issue while testing which I think is unrelated to this PR and submitted a patch which prevents the window from crashing (I think the root cause will be fixed when @bbondy finishes up the tab index PR)

bsclifton · 2017-08-28T20:48:35Z

I went through all 5 issues and added steps / fixed the labels 😄 Should be good to go! Merging 👍

Fixes window restore and fullscreen

NejcZdovc added the refactoring label Aug 13, 2017

NejcZdovc added this to the 0.20.x (Developer Channel) milestone Aug 13, 2017

NejcZdovc self-assigned this Aug 13, 2017

NejcZdovc added the PR/work-in-progress ⚒ label Aug 13, 2017

NejcZdovc force-pushed the refactor/window branch 8 times, most recently from deffe88 to 0a18ce6 Compare August 14, 2017 11:13

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this pull request Aug 14, 2017

Moves createWindow function out of appStore.js

35c0a86

Pre-work for brave#10458 tests Auditors: @bbondy @bsclifton Test Plan: - covered by automated tests

NejcZdovc mentioned this pull request Aug 14, 2017

Moves createWindow function out of appStore.js #10475

Closed

8 tasks

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this pull request Aug 14, 2017

Moves createWindow function out of appStore.js

0d31935

Pre-work for brave#10458 tests Auditors: @bbondy @bsclifton Test Plan: - covered by automated tests

NejcZdovc force-pushed the refactor/window branch from 0a18ce6 to b9a3a8b Compare August 14, 2017 21:34

luixxiul added the needs-info Another team member needs information from the PR/issue opener. label Aug 15, 2017

NejcZdovc removed the needs-info Another team member needs information from the PR/issue opener. label Aug 15, 2017

NejcZdovc force-pushed the refactor/window branch 4 times, most recently from 73f9437 to f7feadb Compare August 17, 2017 18:42

NejcZdovc removed the PR/work-in-progress ⚒ label Aug 17, 2017

NejcZdovc force-pushed the refactor/window branch from f7feadb to 41473e9 Compare August 17, 2017 18:45

NejcZdovc requested review from bsclifton and cezaraugusto August 17, 2017 18:45

This was referenced Aug 18, 2017

When re-launching when maximised, window is offset by several pixels & no longer maximised #7664

Closed

bookmarks not fitting in the bookmark bar #9939

Closed

luixxiul added the misc/fullscreen-option label Aug 20, 2017

NejcZdovc mentioned this pull request Aug 21, 2017

Window off screen when monitors change #3558

Closed

bsclifton suggested changes Aug 23, 2017

View reviewed changes

Removes unused ui data and actions

7e97199

NejcZdovc requested a review from bsclifton August 25, 2017 08:11

NejcZdovc force-pushed the refactor/window branch from 4ff2c50 to aad4a9a Compare August 25, 2017 08:41

Save window appState into windowState when closing a window

921c203

Resolves brave#3754 Resolves brave#6602 Resolves brave#8600 Resolves brave#8925 Resolves brave#9709 Auditors: Test Plan:

NejcZdovc force-pushed the refactor/window branch from aad4a9a to 921c203 Compare August 25, 2017 09:57

Ensure list has a value set before calling push (in FrameStateUtil).

29b8e48

This was causing a crash which breaks the browser window. I believe this issue is related to the issues we have where wrong index is picked after closing / tearing off tab Auditors: @NejcZdovc, @bbondy

bsclifton added the release-notes/include label Aug 28, 2017

bsclifton approved these changes Aug 28, 2017

View reviewed changes

bsclifton merged commit 6fe9849 into brave:master Aug 28, 2017

NejcZdovc pushed a commit that referenced this pull request Aug 28, 2017

Merge pull request #10458 from NejcZdovc/refactor/window

71611d9

Fixes window restore and fullscreen

This was referenced Sep 12, 2017

XY coordinate for the new window when opening needs to offset slightly to make it easy to find multiple windows #10914

Closed

Followup #10605: Fixes window isFocused function that was not updated #10991

Merged

luixxiul mentioned this pull request Sep 30, 2017

Full screen #11216

Closed

NejcZdovc mentioned this pull request Oct 12, 2017

Multiple screens , brave browser out of scope of the screen #9371

Closed

bsclifton mentioned this pull request Oct 16, 2017

Brave forgets that it was maximized #11563

Closed

NejcZdovc mentioned this pull request Oct 18, 2017

Developer tools not re-opening maximized when closed as maximized. #9540

Closed

NejcZdovc mentioned this pull request Oct 27, 2017

Window reopens in the centre of screen when browser is docked to left/right #11697

Closed

luixxiul mentioned this pull request Oct 28, 2017

Download tests are failing on master since 0.20.x #11516

Closed

bsclifton mentioned this pull request Nov 21, 2017

Window positioning is non-existant. #12053

Closed

petemill mentioned this pull request Nov 22, 2017

Hide windows until they are either rendered or a timeout expires #11764

Merged

8 tasks

bsclifton mentioned this pull request Jan 9, 2018

does not remember the full size of the window #12571

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes window restore and fullscreen #10458

Fixes window restore and fullscreen #10458

NejcZdovc commented Aug 13, 2017 •

edited

Loading

codecov-io commented Aug 13, 2017 •

edited

Loading

luixxiul commented Aug 15, 2017

NejcZdovc commented Aug 15, 2017

NejcZdovc commented Aug 15, 2017

bsclifton commented Aug 23, 2017 •

edited

Loading

bsclifton left a comment

NejcZdovc commented Aug 25, 2017

NejcZdovc commented Aug 25, 2017

bsclifton left a comment

bsclifton commented Aug 28, 2017

Fixes window restore and fullscreen #10458

Fixes window restore and fullscreen #10458

Conversation

NejcZdovc commented Aug 13, 2017 • edited Loading

Please don't squash commits

codecov-io commented Aug 13, 2017 • edited Loading

Codecov Report

luixxiul commented Aug 15, 2017

NejcZdovc commented Aug 15, 2017

NejcZdovc commented Aug 15, 2017

bsclifton commented Aug 23, 2017 • edited Loading

bsclifton left a comment

Choose a reason for hiding this comment

NejcZdovc commented Aug 25, 2017

NejcZdovc commented Aug 25, 2017

bsclifton left a comment

Choose a reason for hiding this comment

bsclifton commented Aug 28, 2017

NejcZdovc commented Aug 13, 2017 •

edited

Loading

codecov-io commented Aug 13, 2017 •

edited

Loading

bsclifton commented Aug 23, 2017 •

edited

Loading