New issue
Advanced search Search tips

Issue 916392 link

Starred by 1 user
Status: Fixed
Owner:
dpa...@chromium.org
Closed: Dec 20
Cc:
thestig@chromium.org
dbeam@chromium.org
dpa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Grit: preprocess="true" fails to remove some platform specific code in <include>s

Project Member Reported by rbpotter@chromium.org, Dec 19 
Chrome Version: 73.0.3642.0+

Print Preview was recently updated to use preprocess="true" instead of flattenhtml, see https://chromium-review.googlesource.com/c/chromium/src/+/1377500.

However, platform specific (ios) code from chrome://resources/js/util.js is now being retained in Print Preview's crisper.js, resulting in an extra logging message in the console: "KeyboardEvent.Key polyfill not required".

We should determine what additional updates need to be made to ensure preprocess works correctly for <include>s.

Comment 1 by rbpotter@chromium.org, Dec 19

Summary: Grit: preprocess="true" fails to remove some platform specific code in <include>s (was: Grit: preprocess="true" ignores some platform specific code in <include>s)

Comment 2 by dpa...@chromium.org, Dec 19

Cc: thestig@chromium.org dbeam@chromium.org
Status: Available (was: Untriaged)
Candidate fix https://chromium-review.googlesource.com/c/chromium/src/+/1384662.

Comment 3 by dpa...@chromium.org, Dec 19

Owner: dpa...@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 19 

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

commit 025c99be237efba7ec1f6559d738af488991d7bf
Author: dpapad <dpapad@chromium.org>
Date: Wed Dec 19 22:11:47 2018

GRIT: Fix preprocess=true handling for <include> tags.

Even though the |preprocess| attribute was accepted after r616943,
it was not handled properly.

Also updating tests to actually trigger the relevant codepath.

Bug:  916392 
Change-Id: I1ebfd61f6ab5d244a46863e6e2fddfccf600c099
Reviewed-on: https://chromium-review.googlesource.com/c/1384662
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617973}
[modify] https://crrev.com/025c99be237efba7ec1f6559d738af488991d7bf/tools/grit/grit/node/include.py
[modify] https://crrev.com/025c99be237efba7ec1f6559d738af488991d7bf/tools/grit/grit/node/include_unittest.py

Comment 5 by dpa...@chromium.org, Dec 20

Status: Fixed (was: Started)
►

Sign in to add a comment