Skip to content

Fix #5517: window.open not aware the parameters height and width#77

Closed
xzhan96 wants to merge 2 commits intonwjs:nw21from
xzhan96:bug5517
Closed

Fix #5517: window.open not aware the parameters height and width#77
xzhan96 wants to merge 2 commits intonwjs:nw21from
xzhan96:bug5517

Conversation

@xzhan96
Copy link
Copy Markdown
Contributor

@xzhan96 xzhan96 commented Apr 12, 2017 •
edited
Loading

#5517

@xzhan96
Copy link
Copy Markdown
Contributor Author

xzhan96 commented Apr 12, 2017

@rogerwang please help to review and merge.
#5517

@xzhan96 xzhan96 mentioned this pull request Apr 12, 2017
Closed
@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017

merged into the master patch. thanks.

@rogerwang rogerwang closed this Apr 12, 2017
@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017

The case ' issue4221-win-open-manifest' failed with this patch. Please check it asap.

@xzhan96
Copy link
Copy Markdown
Contributor Author

xzhan96 commented Apr 12, 2017 •
edited
Loading

@rogerwang how to check? what's issue4221-win-open-manifest?

@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017

You should run the sanity test cases as described in the doc: http://docs.nwjs.io/en/latest/For%20Developers/Writing%20Test%20Cases%20for%20NW.js/

I'll revert the patch for now if there is no new patch for this today.

@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017 •
edited
Loading

To run a single case, simply set the environment variable CHROMEDRIVER to the full path of chromedriver executable in the build output directory and set PYTHONPATH to the full path of src/third_party/webdriver/pylib/; chdir to the case directory and run python test.py

@xzhan96
Copy link
Copy Markdown
Contributor Author

xzhan96 commented Apr 12, 2017

@rogerwang the patch is correct. The issue is in the test case.
Before the patch, the new window created by window.open will always has the same width/height with main window, and the test just check if(new window's size == main window's size), so the test can be passed.
However, that't not right, per the description in nw doc, the width/height in package.json is the initial width/height of the main window.

So the test case need to be updated.

@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017 via email

@xzhan96
Copy link
Copy Markdown
Contributor Author

xzhan96 commented Apr 12, 2017

sure, I will submit a new PR for that.

@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017 via email

@rogerwang
Copy link
Copy Markdown
Member

rogerwang commented Apr 12, 2017

I checked it again. According to the document, most of window subfields should be inherited by sub windows opened by window.open() or links by default, including height/width. So the test case is correct.

@xzhan96 xzhan96 deleted the bug5517 branch April 13, 2017 02:33
GnorTech pushed a commit that referenced this pull request Apr 26, 2017
cros: Mount the existing eCryptfs vault to start migration.
268dd43 
Before we call cryptohome's MigrateToDircrypto method, we need to mount
existing eCryptfs valut to start migration by calling MountEx method with
|to_migrate_from_ecryptfs| mount flag.

Additionally, this CL removes |auth| argument from MigrateToDircrypto method,
since it was removed from DBUS call in the latest cryptohome.
https://chromium-review.googlesource.com/#/c/472409/6/cryptohome/cryptohome.xml

BUG=711458
TBR=xiyuan@chromium.org,hashimoto@chromium.org

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

Review-Url: https://codereview.chromium.org/2827203002 .
Cr-Commit-Position: refs/branch-heads/3071@{#77}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
fujunwei pushed a commit to fujunwei/chromium.src that referenced this pull request May 22, 2017
Incrementing VERSION to 58.0.3029.9
c71044e 
Cr-Commit-Position: refs/branch-heads/3029@{nwjs#77}
Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
GnorTech pushed a commit that referenced this pull request Aug 5, 2017
VR: Add offline mode URL rendering unit test.
aeeab44 
Check that:
- The security verbose text is present in offline mode oly.
- The URL moves when security verbose text is present.

BUG=
TBR=cjgrant@chromium.org

(cherry picked from commit d4a941c)

Change-Id: I2220ba6a2657a78a7fab1cb03fd23e6b02024bf9
Reviewed-on: https://chromium-review.googlesource.com/582934
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489020}
Reviewed-on: https://chromium-review.googlesource.com/588849
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#77}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
GnorTech pushed a commit that referenced this pull request Sep 14, 2017
@reveman-chromium
login: Don't discard old default user image
7ded924 
This adds support for syncing of image index outside current set
and updates the user image screen to support old images.

TBR=reveman@chromium.org

(cherry picked from commit c5d6a1e)

Bug: 761665
Test: old default user image is available on user image screen.
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Id3952955f6114420e122ac6171fedf5ca2dba910
Reviewed-on: https://chromium-review.googlesource.com/648807
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499862}
Reviewed-on: https://chromium-review.googlesource.com/656677
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#77}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
GnorTech pushed a commit that referenced this pull request Oct 27, 2017
@dschuyler
[MD settings] fix scrolling in siteData page.
2f1546d 
This CL applies GlobalScrollTargetBehavior to the siteData iron-list so
that the list only shows one scroll bar (rather than two) and the list
scrolls properly.

Bug: 775215
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I7b21f099446dabdf3e356dd66898352d8bc73610
Reviewed-on: https://chromium-review.googlesource.com/722340
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509480}(cherry picked from commit 13ea87b)
Reviewed-on: https://chromium-review.googlesource.com/728402
Cr-Commit-Position: refs/branch-heads/3239@{#77}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
rogerwang pushed a commit that referenced this pull request Jan 29, 2018
Fail the web app manifest fetch if the document is sandboxed.
c5c4999 
This ensures that sandboxed pages are regarded as non-PWAs, and that
other features in the browser process which trust the web manifest do
not receive the manifest at all if the document itself cannot access the
manifest.

BUG=771709

Change-Id: Ifd4d00c2fccff8cc0e5e8d2457bd55b992b0a8f4
Reviewed-on: https://chromium-review.googlesource.com/866529
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531121}(cherry picked from commit ffac0ee)
Reviewed-on: https://chromium-review.googlesource.com/884921
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#77}
Cr-Branched-From: bc084a8-refs/heads/master@{#530369}
GnorTech pushed a commit that referenced this pull request Mar 20, 2018
@jhorwich
Record ARC consents using consent_auditor
ed9cd70 
Record ARC-related consents using consent_auditor. This includes
Play ToS, Backup and Restore, and Location Services.

Adds checks to existing unit tests for proper consent recording, as
well as adding a unit test to cover a hybrid managed/unmanaged case.

Also fix a trivial bug in arc_terms_of_service.js referring to an
old name for a method in ArcTermsOfServiceScreen.

Bug: 781765
Test: Opt in on device, run unit_tests
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ibce048770f0479c2c87abd08dcfaf1fb19bf608c
Reviewed-on: https://chromium-review.googlesource.com/930473
Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Achuith Bhandarkar <achuith@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#540661}(cherry picked from commit 37d7f73)
Reviewed-on: https://chromium-review.googlesource.com/953822
Reviewed-by: Josh Horwich <jhorwich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#77}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
rogerwang pushed a commit that referenced this pull request Apr 25, 2018
Reland "VR: Clean up WebVR headset insertion."
310c5de 
This reverts commit 30b94a8.

Original change's description:
> Revert "VR: Clean up WebVR headset insertion."
>
> This reverts commit f2ccbfd.
>
> Reason for revert: Speculative revert for crbug.com/831589
>
> Original change's description:
> > VR: Clean up WebVR headset insertion.
> >
> > This CL makes it explicitly clear when we care about whether the page
> > was listening for activate before pause, and when we should autopresent
> > on entering VR (for headset insertion).
> >
> > This also removes the workaround in vr_display_impl that allowed pages
> > without focus to handle displayActivate, favoring handling this case
> > more correctly in VrShellDelegate.
> >
> > Bug: 829513
> > Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
> > Reviewed-on: https://chromium-review.googlesource.com/1004927
> > Reviewed-by: Yash Malik <ymalik@chromium.org>
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#549736}
>
> TBR=mthiesse@chromium.org,ymalik@chromium.org
>
> Change-Id: I92a56254e933381770a56495553facfbcb2e5c59
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 829513
> Reviewed-on: https://chromium-review.googlesource.com/1007374
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549886}

TBR=bajones@chromium.org, mthiesse@chromium.org

(cherry picked from commit 40a37bc)

Change-Id: I84fa5bd2f187859770166c1a3c79387492b50df1
Bug: 829513
Reviewed-on: https://chromium-review.googlesource.com/1007922
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550039}
Reviewed-on: https://chromium-review.googlesource.com/1016686
Cr-Commit-Position: refs/branch-heads/3396@{#77}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
rogerwang pushed a commit that referenced this pull request Jun 19, 2018
(merge) Improve list of display zoom values
152762c 
This patch changes the logic the list of zoom values are computed for a
given display. The new logic has 2 scenarios for listing the zoom values
for a given display.

1) Displays with device scale factors assigned to them, will now have
   zoom values ranging from the inverse of device scale factor to
   device scale factor. If there are still slider ticks avaiable we use
   them to add zoom levels beyond the device scale factor.
   Doing this allows the user to go to the native resolution of the
   display and on the other hand it also allows them to zoom in if
   required. We no longer allow the user to go to a zoom below the native
   resolution of the display as this is a very unlikely scenario and
   introduces artifacts. This also gives finer control to the user in
   setting the zoom level.
   How this effects a pixelbook for example?
   On pixelbook we used to have a zoom range of 50% to 175%. Due to the
   wide range, the consecutive values were too far apart and users wanted
   zoom values that were mostly in the range of 70% to 100%.
   With the new change, the range will go from 50% to 130%.

2) Displays with no device scale factors assigned to them will use a
   static list of initialized zoom values.

Bug: 845634
Change-Id: I69a761856dab4e5b37b85420f6f6dfebdb2dead5
Component: Display zoom, display util
Reviewed-on: https://chromium-review.googlesource.com/1069561
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563050}(cherry picked from commit 28cc23e)
Reviewed-on: https://chromium-review.googlesource.com/1081688
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#77}
Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
GnorTech pushed a commit that referenced this pull request Aug 4, 2018
@strangewiz
[ios] Clean up NTP focus animation.
1881830 
Previously the NTP 'scroll up' animation and the omnibox 'focus' animation happened
concurrently when tapping on the NTP fakebox.  Instead, chain these animations, so
the NTP fakebox 'scroll up' animations completes and then focuses the real omnibox.

Also changes how the 'scroll up' animation works to use a CADisplayLink, so each
tick of the scroll updates the fakebox shape and hint label size.

See: https://drive.google.com/open?id=11dbkXTl0V0n5WODLiBYezpdkH-eUvxKO

Bug: 865408, 865822
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I85d8d193178602a434e4f0007513c38be69e9fec
Reviewed-on: https://chromium-review.googlesource.com/1142946
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577086}(cherry picked from commit a00d307)
Reviewed-on: https://chromium-review.googlesource.com/1150342
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#77}
Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
rogerwang pushed a commit that referenced this pull request Sep 15, 2018
@strangewiz
[ios] Re-use grid_cell_close_button in tab strip.
dc11237 
Removes similar tabstrip_tab_close_* images, and instead uses
a tint and a HighlightedButton.

Bug: 876881

Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ief49d6594b7725a13f852903b64cf2af843497d4
Reviewed-on: https://chromium-review.googlesource.com/1197502
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588679}(cherry picked from commit 9a5ce67)
Reviewed-on: https://chromium-review.googlesource.com/1208419
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#77}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
rogerwang pushed a commit that referenced this pull request Oct 31, 2018
Download home v2: Fix issue in storage summary.
66d9ab0 
Currently removing an ongoing download may hit assertion in
StorageSummaryProvider, this is due to a bug in onItemUpdated that we
increase the total storage based on old item instead of the new item.

TBR=xingliu@chromium.org

(cherry picked from commit 12be1cb)

Bug: 892328
Change-Id: I64cda81662290df049f74aa3f1893c7a0984c043
Reviewed-on: https://chromium-review.googlesource.com/c/1284705
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600167}
Reviewed-on: https://chromium-review.googlesource.com/c/1285354
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#77}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Dec 18, 2018
[iOS][MF] Enable pull to reset
69e568c 
Similar to pull to refresh, pulling the suggestions to the right will
reset the keyboard and the manual fallback view.

Video: https://drive.google.com/file/d/1QpoiU3YPXdFhLQ_ubzZcyJ7VxVo86Zpv/view?usp=sharing

Bug: 845472, 911142
Change-Id: Ie13322d168dac81288d95a8e10a0bcb5bab59339
Reviewed-on: https://chromium-review.googlesource.com/c/1355188
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613169}(cherry picked from commit 30956fc)
Reviewed-on: https://chromium-review.googlesource.com/c/1363197
Reviewed-by: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#77}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
rogerwang pushed a commit that referenced this pull request Feb 10, 2019
Preserve vector order in RemoveDuplicatePrepopulateIDs
e7804de 
This CL is the union of 2 other CLs. The purpose of this CL is
to ensure one clean merge to the M73 branch. The 2 source CLs are:

----- https://crrev.com/c/1428939
Preserve vector order in RemoveDuplicatePrepopulateIDs

Template URLs with nonzero prepopulate_id were implicitly
reordered by RemoveDuplicatePrepopulateIDs, though relative
order was preserved for elements with zero prepopulate_id.
This CL adjusts the algorithm to output the selected
representatives for each prepopulate_id in order of first
appearance in the original template_urls vector, stabilizing
the overall order (instead of partitioning as it did before).

Bug: 924268
Change-Id: Ic885065cbf20398dfff723a85aff35141e0cb9a5
Reviewed-on: https://chromium-review.googlesource.com/c/1428939
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626208}(cherry picked from commit ceb5964)

----- https://crrev.com/c/1437703
Add includes for IWYU: follow-on for cl/1428939

This CL just adds some includes that were harmlessly forgotten
on https://crrev.com/c/1428939 .

Change-Id: Ic885065cbf20398dfff723a85aff35141e0cb9a5
Reviewed-on: https://chromium-review.googlesource.com/c/1444696
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#77}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
rogerwang pushed a commit that referenced this pull request Mar 24, 2019
@shuchen-google
Fix the bad rule patterns in VNI.
2cf6e82 
".*" is too aggressive and caused the prefix matching always return true.
This cl safely replaces ".*" with "\w*".

Bug: 940338
Change-Id: Iedc8e2a1c61b43959310024d980b98c715b5bffb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1514348
Commit-Queue: Leo Zhang <googleo@chromium.org>
Reviewed-by: Leo Zhang <googleo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#639417}(cherry picked from commit 7fa1bcd2bed8425b687e2e0ae74f9c2ee5a91482)
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1520507
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#77}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rogerwang rogerwang rogerwang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xzhan96 @rogerwang