New issue
Advanced search Search tips

Issue 678778 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 567770



Sign in to add a comment

GRIT: put <if> and <include> behind comments in .js files to make syntactically valid JS

Project Member Reported by dbeam@chromium.org, Jan 5 2017

Issue description

.js files in Chrome can have <if> and <include> in them, for example

<include src="some-file.js">

function isChromeOs() {
<if expr="chromeos">
  return true;
</if>
<if expr="not chromeos">
  return false;
</if>
}

useMethodFromSomeFile(isChromeOs());

this is similar to #if and #include in C++.

unfortunately, many tools designed to read .js files as JavaScript blow up on this syntax.  closure compiler, closure linter, clang-format, etc (to name a few).

we should put these directives behind comments (which has no effect on GRIT) so that the tools that parse .js files have valid JavaScript to work with.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42fdc659ccc288d4a3a01f202c7d667e76b04192

commit 42fdc659ccc288d4a3a01f202c7d667e76b04192
Author: dbeam <dbeam@chromium.org>
Date: Fri Jan 06 02:53:00 2017

Fix grit's <if> not to strip trailing or preceding whitespace

This is so a single line comment can be used to exclude <if> in .js
files. I've added tests for both <include> and <if>. They need to be
commented for clang-format purposes.

BUG= 678778 
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2615953002
Cr-Commit-Position: refs/heads/master@{#441853}

[modify] https://crrev.com/42fdc659ccc288d4a3a01f202c7d667e76b04192/tools/grit/grit/format/html_inline.py
[modify] https://crrev.com/42fdc659ccc288d4a3a01f202c7d667e76b04192/tools/grit/grit/format/html_inline_unittest.py

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94146b3fa89acb43b24182b30258f3fb5bb62047

commit 94146b3fa89acb43b24182b30258f3fb5bb62047
Author: dbeam <dbeam@chromium.org>
Date: Mon Jan 09 20:16:41 2017

GRIT: put <if> and <include> behind comments in .js files to make syntactically valid JS

This unblocks clang-format on .js files. See bug for more details.

R=thakis@chromium.org,hirono@chromium.org,xiyuan@chromium.org
TBR=jam@chromium.org
BUG= 678778 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2617823002
Cr-Commit-Position: refs/heads/master@{#442322}

[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/first_run/first_run.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/keyboard_overlay.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/custom_elements_login.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/custom_elements_oobe.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/lock.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/login.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/login_non_lock_shared.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/login_shared.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/oobe.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/chromeos/wallpaper_manager/js/main_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/extensions/chromeos/kiosk_apps.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/extensions/extension_command_list.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/extensions/extension_commands_overlay.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/extensions/extension_list.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/extensions/extensions.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/feedback/js/event_handler.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/feedback/js/feedback.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/gaia_auth_host/authenticator.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/gaia_auth_host/post_message_channel.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/gaia_auth_host/saml_handler.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/gaia_auth_host/webview_saml_injected.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/help/help.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/help/help_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/history/history.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/local_ntp/most_visited_util.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/md_user_manager/user_manager.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/media_router/media_router.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/options/browser_options.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/options/options.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/options/options_bundle.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/pdf/elements/viewer-page-indicator/viewer-page-indicator.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/predictors/predictors.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/a11y_page/a11y_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/about_page/about_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/about_page/about_page_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/appearance_page/appearance_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/downloads_page/downloads_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/languages_page/languages_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/lifetime_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/people_page/people_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/people_page/pin_keyboard.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/people_page/sync_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/printing_page/printing_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/privacy_page/privacy_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/privacy_page/privacy_page_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/reset_page/reset_browser_proxy.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/reset_page/reset_page.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/route.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/settings/settings_ui/settings_ui.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/chrome/browser/resources/uber/uber_frame.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/components/chrome_apps/webstore_widget/app/main.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/components/neterror/resources/neterror.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/components/security_interstitials/core/browser/resources/interstitial_v2.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/content/browser/resources/gpu/gpu_internals.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/content/browser/resources/media/media_internals.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/content/browser/resources/media/stats_graph_helper.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/content/browser/resources/media/webrtc_internals.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/audio_player/js/audio_player_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/audio_player/js/background_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/file_manager/background/js/background_common_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/file_manager/background/js/background_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/gallery/js/background_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/gallery/js/gallery_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/image_loader/background_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/video_player/js/background_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/file_manager/video_player/js/video_player_scripts.js
[modify] https://crrev.com/94146b3fa89acb43b24182b30258f3fb5bb62047/ui/webui/resources/js/cr/ui/array_data_model.js

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d4982588b9a0dc7a6685e850235dbe394eb7731

commit 1d4982588b9a0dc7a6685e850235dbe394eb7731
Author: dbeam <dbeam@chromium.org>
Date: Thu Jan 12 00:12:01 2017

Add web_dev_style presubmit to ensure <if>/<include> live in comments

Before:

<include src="...">
<if expr="chromeos">
    chromeOsOnlyCode();
</if>

After:

// <includes src="...">
// <if expr="chromeos">
    chromeOsOnlyCode();
// </if>

This is to unblock running clang-format on JavaScript and allow other
tools to run on more expected input inside of .js files (i.e. closure
compiler).

BUG= 678778 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2624503002
Cr-Commit-Position: refs/heads/master@{#443069}

[modify] https://crrev.com/1d4982588b9a0dc7a6685e850235dbe394eb7731/chrome/browser/resources/settings/people_page/people_page.js
[modify] https://crrev.com/1d4982588b9a0dc7a6685e850235dbe394eb7731/chrome/browser/web_dev_style/js_checker.py
[modify] https://crrev.com/1d4982588b9a0dc7a6685e850235dbe394eb7731/chrome/browser/web_dev_style/js_checker_test.py

Comment 4 by dbeam@chromium.org, Jan 12 2017

Cc: dmazz...@chromium.org
Status: Fixed (was: Started)

Sign in to add a comment