KEMBAR78
Enhance chrome.app.window API with window background color by fujunwei · Pull Request #70 · nwjs/chromium.src · GitHub
Skip to content

Conversation

@fujunwei
Copy link
Contributor

@fujunwei fujunwei commented Mar 7, 2017

The default background color of window is white, a flash of white
will be shown if the loaded page uses a dark background. So add a new
property named background_color to CreateWindowOptions that can be configured
the same as page dark background instead of white default color.

BUG=nwjs/nw.js#5502

@fujunwei
Copy link
Contributor Author

fujunwei commented Mar 8, 2017

@rogerwang PTAL, thanks.

Copy link
Member

@rogerwang rogerwang left a comment

Choose a reason for hiding this comment

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

I'll see it after 0.21.0 release, which will happen soon.

Please rebase with latest nw21 branch and add documentation in the nwjs repo.

@rogerwang
Copy link
Member

btw, you might be interested in this: https://codereview.chromium.org/2733093002

@fujunwei
Copy link
Contributor Author

fujunwei commented Mar 9, 2017

I will send the PR of first started white flash to chromium upstream at the same time. Thanks.

@rogerwang rogerwang force-pushed the nw21 branch 2 times, most recently from c286db2 to aa1726c Compare March 10, 2017 05:25
rogerwang pushed a commit that referenced this pull request Mar 11, 2017
Get rid of colon in Autofill credit card popup suggestions last used
date UI string.

BUG=698391
TEST=CreditCardTest.GetLastUsedDateForDisplay(components_unittests)

Review-Url: https://codereview.chromium.org/2730133003
Cr-Commit-Position: refs/heads/master@{#455533}
(cherry picked from commit e5e6dd9)

Review-Url: https://codereview.chromium.org/2739743003 .
Cr-Commit-Position: refs/branch-heads/3029@{#70}
Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
@rogerwang rogerwang force-pushed the nw21 branch 3 times, most recently from 65c8d20 to 482e210 Compare March 12, 2017 13:59
@rogerwang
Copy link
Member

please rebase

@rogerwang rogerwang force-pushed the nw21 branch 8 times, most recently from 426fc70 to f78c16a Compare March 21, 2017 05:50
@rogerwang rogerwang force-pushed the nw21 branch 3 times, most recently from da3929b to b060cdf Compare March 28, 2017 04:57
@halton
Copy link
Contributor

halton commented Jun 8, 2017

@fujunwei I guess you finish the rebase, is ready for @rogerwang's review?

@fujunwei
Copy link
Contributor Author

fujunwei commented Jun 9, 2017

Yes, it's ready. @rogerwang do you have other comments?

@rogerwang
Copy link
Member

In our previous meeting in May, we talked about a patch you submitted to upstream for this feature. I asked for the link to be posted in this PR so I can check upstream's comments. And you didn't answer my question in another PR you submitted 2 weeks ago with the same title: #82 (comment)

@fujunwei
Copy link
Contributor Author

fujunwei commented Jun 9, 2017

In our previous meeting in May, we talked about a patch you submitted to upstream for this feature. I asked for the link to be posted in this PR so I can check upstream's comments.

https://codereview.chromium.org/2738373002 is propose to enhance chrome.app.window API with an option of background color, but app approver considers it’s not necessary to a background color property that discusses in https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/chromium-dev/xkZX6YhBuJo/Vt-iwy1lBAAJ.

so i sent another cl to fix https://bugs.chromium.org/p/chromium/issues/detail?id=470669 that is a white flash in the content area while the NTP waits to load/paint. CL is https://codereview.chromium.org/2820753005, but the issue was resolved by someone's CL.

but cl https://codereview.chromium.org/2820753005 is going on discussing with upstream.

And you didn't answer my question in another PR you submitted 2 weeks ago with the same title: #82 (comment)

I saw you have closed #82 that is based on nw22, so i didn't continue to give comments.

@rogerwang
Copy link
Member

So why did you launch another PR while this one is open and under review? Are they with same code? Which one should I review?

@fujunwei
Copy link
Contributor Author

fujunwei commented Jun 9, 2017

Yes, the same. Please continue reviewing this PR that base on nw21.

GnorTech pushed a commit that referenced this pull request Aug 5, 2017
The data saver menu footer is supposed to be highlighted when the data
saver in-product help is shown but it is not. This patch fixed it by
changing the ID that is highlighted to be the inflatedID of the app menu footer
view stub.

BUG=747486

(cherry picked from commit c338ce8)

Change-Id: Iaf43badcf277dd424fc02fbafa4d27e51c03737c
Reviewed-on: https://chromium-review.googlesource.com/581879
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Angela Shao <angelashao@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#488785}
Reviewed-on: https://chromium-review.googlesource.com/588247
Cr-Commit-Position: refs/branch-heads/3163@{#70}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
GnorTech pushed a commit that referenced this pull request Sep 14, 2017
Currently Play Store app search results will remain in the launcher
forever if Android crashes or reboots. This commit clears the list
once ArcPlayStoreSearchProvider fails to communicate to Android.

TBR=hejq@google.com

(cherry picked from commit 5cbb0fe)

Bug: 762121
Change-Id: Idd9f889015e4f70a8fb1f8ae96c11076ad86511c
Reviewed-on: https://chromium-review.googlesource.com/651186
Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Jiaquan He <hejq@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#499726}
Reviewed-on: https://chromium-review.googlesource.com/656317
Reviewed-by: Vadim Tryshev <vadimt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#70}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
GnorTech pushed a commit that referenced this pull request Oct 27, 2017
Error and warning counters show up by default, even when console has
no messages of those types. The counter logic already calls _update()
initially, but it results in a no-op when error/warningCount are
up-to-date by default.

This CL initializes error/warningCount differently to ensure the first
_update() will not be a no-op.

Bug: 775239
Change-Id: I98f3c1aa0838baf70c3372c4a07781011878f8f0
Reviewed-on: https://chromium-review.googlesource.com/722077
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509560}(cherry picked from commit eda21d3)
Reviewed-on: https://chromium-review.googlesource.com/728401
Reviewed-by: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#70}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
rogerwang pushed a commit that referenced this pull request Jan 29, 2018
This patch updates the externs for 'system_display.js' for display zoom
factor.
This is a follow up patch to
https://chromium-review.googlesource.com/c/chromium/src/+/864831

Bug: 790723
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I629f333d0965ad34e34c9d83adbffef1466fa17a
Component: system display, extern, IDL
Reviewed-on: https://chromium-review.googlesource.com/879263
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531082}(cherry picked from commit a8ddd80)
Reviewed-on: https://chromium-review.googlesource.com/883949
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#70}
Cr-Branched-From: bc084a8-refs/heads/master@{#530369}
GnorTech pushed a commit that referenced this pull request Mar 20, 2018
Bug: 819295
Change-Id: I0600ca9fc724813743672c83b27e9adf0dcb28a2
Reviewed-on: https://chromium-review.googlesource.com/946518
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/953362
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#70}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
rogerwang pushed a commit that referenced this pull request Apr 25, 2018
This fix the issue when default ARC app icons are not loaded in case
of provisioning is not finished for new user flow.

TBR=khmel@google.com

(cherry picked from commit aa4e005)

Test: Manually + unit_test
Bug: 833596
Change-Id: I99af81eafb78c65857b6fe5a6475d3372e08beb5
Reviewed-on: https://chromium-review.googlesource.com/1014326
Commit-Queue: Yury Khmel <khmel@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551182}
Reviewed-on: https://chromium-review.googlesource.com/1016121
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#70}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
rogerwang pushed a commit that referenced this pull request Jun 19, 2018
Updates the skip-ahead logic in audio::LoopbackStream to skip forward by
one more buffer. Because of integer truncation/rounding issues when
converting between units (samples→microseconds), the previous logic
might occasionally not skip-ahead far enough in time. In these cases,
the next buffer generation time would be a point-in-time a few
microseconds in the past, which would rightly trigger a DCHECK().

Bug: 847487
Change-Id: I1b8495e4b9dc71c95ad5bcc6ee5dd252dd54a364
Reviewed-on: https://chromium-review.googlesource.com/1077613
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#562692}(cherry picked from commit a157e95)
Reviewed-on: https://chromium-review.googlesource.com/1081475
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#70}
Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
GnorTech pushed a commit that referenced this pull request Aug 4, 2018
BUG=866241
TBR=junov@chromium.org

(cherry picked from commit bbe5f56)

Change-Id: Ic789573da753fe1e438778d777556a659fbf0ba9
Reviewed-on: https://chromium-review.googlesource.com/1147460
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577322}
Reviewed-on: https://chromium-review.googlesource.com/1150207
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#70}
Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
rogerwang pushed a commit that referenced this pull request Sep 15, 2018
…ral users

TBR=wzang@chromium.org

(cherry picked from commit 3279bfd)

Bug: 879310
Change-Id: I65ac6b822e1fadd9440189a3e11c6d4fa118ec8f
Reviewed-on: https://chromium-review.googlesource.com/1198762
Commit-Queue: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#587934}
Reviewed-on: https://chromium-review.googlesource.com/1208813
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#70}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
rogerwang pushed a commit that referenced this pull request Oct 31, 2018
- Adds a message to provide users with a more detailed error message
  when SMB mounting to a file server that doesn't support SMBv2 or later.

Bug: chromium:887127
Change-Id: I9e2ff3c278bb014c5538082ddcba42c8556ba8f2
Reviewed-on: https://chromium-review.googlesource.com/c/1274608
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599353}(cherry picked from commit a44b1ca)
Reviewed-on: https://chromium-review.googlesource.com/c/1285088
Reviewed-by: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#70}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Dec 18, 2018
…on settings

Show warning icon on public session login screen when managed
sessions are enabled.


(cherry picked from commit f99f63b)

Bug: 910218
Change-Id: I119dd321c6d9a7bf8dd3b3da533b8774e9620bc7
Reviewed-on: https://chromium-review.googlesource.com/c/1337492
Commit-Queue: Zakhar Voit <voit@google.com>
Reviewed-by: Ivan Šandrk <isandrk@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613352}
Reviewed-on: https://chromium-review.googlesource.com/c/1362894
Cr-Commit-Position: refs/branch-heads/3626@{#70}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
rogerwang pushed a commit that referenced this pull request Feb 10, 2019
Media Session API should not be available in Android
WebView so we should update expectations.

BUG=925700

Change-Id: I52330a6fa1d42501ea4bc75e32ddde26db349e72
Reviewed-on: https://chromium-review.googlesource.com/c/1440908
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#627043}(cherry picked from commit 87c33c5)
Reviewed-on: https://chromium-review.googlesource.com/c/1446397
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#70}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
rogerwang pushed a commit that referenced this pull request Mar 24, 2019
This reverts commit 7dcbfa1.

Reason for revert: breaks build, bug 941226

Original change's description:
> Use ObserverList for OverviewModeBehavior
> 
> This switches to use ObserverList
> OverviewModeObservers in ChromeTabbedActivity. This list gets frequently
> modified during iteration, and Array List is throwing a
> ConcurrentModificationException because of this right now.
> 
> BUG=938236
> TBR=yusufo@google.com
> 
> (cherry picked from commit 0775dc30ab0ccfd38181c219c3bc725ae03a5fa4)
> 
> Change-Id: I65c99b774ae34f2984ddb8f5ca254ff8e2af4569
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1515713
> Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#639660}
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1518364
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3729@{#63}
> Cr-Branched-From: d4a8972-refs/heads/master@{#638880}

TBR=tedchoc@chromium.org,yusufo@chromium.org

Change-Id: I20a905b15643de0cc270a5c91dcf79f4a81d9c6d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 938236
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1519449
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#70}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
rogerwang pushed a commit that referenced this pull request Mar 24, 2019
This reverts commit 4bc7b1c.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Revert "Use ObserverList for OverviewModeBehavior"
> 
> This reverts commit 7dcbfa1.
> 
> Reason for revert: breaks build, bug 941226
> 
> Original change's description:
> > Use ObserverList for OverviewModeBehavior
> > 
> > This switches to use ObserverList
> > OverviewModeObservers in ChromeTabbedActivity. This list gets frequently
> > modified during iteration, and Array List is throwing a
> > ConcurrentModificationException because of this right now.
> > 
> > BUG=938236
> > TBR=yusufo@google.com
> > 
> > (cherry picked from commit 0775dc30ab0ccfd38181c219c3bc725ae03a5fa4)
> > 
> > Change-Id: I65c99b774ae34f2984ddb8f5ca254ff8e2af4569
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1515713
> > Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
> > Reviewed-by: Ted Choc <tedchoc@chromium.org>
> > Cr-Original-Commit-Position: refs/heads/master@{#639660}
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1518364
> > Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> > Cr-Commit-Position: refs/branch-heads/3729@{#63}
> > Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
> 
> TBR=tedchoc@chromium.org,yusufo@chromium.org
> 
> Change-Id: I20a905b15643de0cc270a5c91dcf79f4a81d9c6d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 938236
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1519449
> Reviewed-by: Krishna Govind <govind@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3729@{#70}
> Cr-Branched-From: d4a8972-refs/heads/master@{#638880}

TBR=tedchoc@chromium.org,yusufo@chromium.org,govind@chromium.org

Change-Id: I9a6ddeb86be7420426867d52706ac53712d0f482
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 938236
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1521954
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#105}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
@ParticleCore
Copy link

Is there any progress on this option? (Adding the ability to set the default browser background color) It's been two years.

@fujunwei
Copy link
Contributor Author

It's unnecessary no longer.

@fujunwei fujunwei closed this Mar 31, 2019
@ParticleCore
Copy link

@fujunwei I cannot see it in the documentation, has it been implemented in a different way? Because it is still showing a white flash when the app is launched before any CSS/HTML has a chance to load. This is on a Windows 10 running the nwjs-sdk-v0.37.1-win-x64

@lst1975
Copy link

lst1975 commented Apr 28, 2019

first, in the file package.json, window: { show: false }
second, body <style='background-color:xxx'>
finally, when index.html loaded, run the following code
require('nw.gui').Window.get().show;

@ParticleCore
Copy link

@lst1975 That is not a solution for the problem, it is a workaround that has been in use since this issue was first brought to their attention. This will cause the app to appear to be "stuck" or lag whenever it is opened, it is not a good experience to the user, you have to remember to do that for every single window you create and if it isn't a window with nw context you cannot do this at all.

@lst1975
Copy link

lst1975 commented Apr 30, 2019 via email

@zydrone
Copy link

zydrone commented Oct 25, 2021

I'm having this problem as well. Tested in nw.js 29, 30 and 57.
I'm pretty sure this is a Chromium issue, so not blaming nw.js.

Seems that having the window "show":false property set, and then calling NW.Window.get().show() does not fix the issue.
It still flashes white when opening the window, under Windows 10.

Therefore I was happy to see this "background_color" commit, which seems like a simple fix.
However, I fail to see this documented anywhere, nor working.
Can you tell me which version(s) of NW.js have this feature?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants