Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1967743002:
Temp fix for blank view. Somehow the third calls to AddIconToLayout() cause this strange bug. Tried… (Closed)
DescriptionTemp fix for blank view. Somehow the third calls to AddIconToLayout() cause this strange bug. Tried to address this by remember the Skia in global variable but neither that work. This is a temp fix till we figure out what is going on in the ImageView.
BUG=610351
Committed: https://crrev.com/1a26f1f5d6e4ecaa7f098302b17d8f87eb690362
Cr-Commit-Position: refs/heads/master@{#392761}
Patch Set 1 #Patch Set 2 : add comments adn also remove the icon from the error view #
Total comments: 3
Patch Set 3 : modify comments to TODO(ftang) form #Messages
Total messages: 20 (7 generated)
The CQ bit was checked by ftang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967743002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967743002/1
add comments adn also remove the icon from the error view
The CQ bit was checked by ftang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967743002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967743002/20001
ftang@chromium.org changed reviewers: + msw@chromium.org
Somehow when we show icon in three views, the third view became blank. I cannot figure out what is going on under the hood. This is a temp fix to make the feature "usable" until we figure out why the third view with icon will show up blank (either AfterTranslteView or the ErrorView).
Is the new UI on-by-default? https://codereview.chromium.org/1967743002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/translate/translate_bubble_view.cc (right): https://codereview.chromium.org/1967743002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/translate/translate_bubble_view.cc:88: // Somehow the third views calling AddIconToLayout show blank as in This comment isn't very good. What does "third views" mean? Use the more common 'crbug.com' url, s/hook/hood/, who is 'we', and what do you mean by caching in skia (caching what?) and what global variable did you try? Instead, consider saying something like this: // TODO(ftang) Restore icons in CreateViewAfterTranslate and CreateViewError // without causing layout issues; see http://crbug.com/610351 Or instead put two comments like this in the respective functions: // TODO(ftang) Restore icon without causing layout defects: crbug.com/610351
modify comments to TODO(ftang) form
The CQ bit was checked by ftang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967743002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967743002/40001
No, the New UI is not on the default yet. You need to see it by visiting chrome://flags and change TranslateUI2016Q2 to "Enabled" to see it (by visiting a foreign language page) PTAL https://codereview.chromium.org/1967743002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/translate/translate_bubble_view.cc (right): https://codereview.chromium.org/1967743002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/translate/translate_bubble_view.cc:88: // Somehow the third views calling AddIconToLayout show blank as in On 2016/05/10 20:43:21, msw wrote: > This comment isn't very good. What does "third views" mean? Any third view (regardless which one) show the icon. Somehow there are no problem for the first two views (regardless which two) to show the icon but the third view (regardless which one) add the icon will cause it to become blank. > Use the more common 'crbug.com' url, s/hook/hood/, who is 'we', and what do you mean by caching in > skia (caching what?) and what global variable did you try? > > Instead, consider saying something like this: > // TODO(ftang) Restore icons in CreateViewAfterTranslate and CreateViewError > // without causing layout issues; see http://crbug.com/610351 > > Or instead put two comments like this in the respective functions: > // TODO(ftang) Restore icon without causing layout defects: crbug.com/610351 https://codereview.chromium.org/1967743002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/translate/translate_bubble_view.cc:88: // Somehow the third views calling AddIconToLayout show blank as in On 2016/05/10 20:43:21, msw wrote: > This comment isn't very good. What does "third views" mean? Use the more common > 'crbug.com' url, s/hook/hood/, who is 'we', and what do you mean by caching in > skia (caching what?) and what global variable did you try? > > Instead, consider saying something like this: > // TODO(ftang) Restore icons in CreateViewAfterTranslate and CreateViewError > // without causing layout issues; see http://crbug.com/610351 > > Or instead put two comments like this in the respective functions: > // TODO(ftang) Restore icon without causing layout defects: crbug.com/610351 Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ftang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967743002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967743002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Temp fix for blank view. Somehow the third calls to AddIconToLayout() cause this strange bug. Tried to address this by remember the Skia in global variable but neither that work. This is a temp fix till we figure out what is going on in the ImageView. BUG=610351 ========== to ========== Temp fix for blank view. Somehow the third calls to AddIconToLayout() cause this strange bug. Tried to address this by remember the Skia in global variable but neither that work. This is a temp fix till we figure out what is going on in the ImageView. BUG=610351 Committed: https://crrev.com/1a26f1f5d6e4ecaa7f098302b17d8f87eb690362 Cr-Commit-Position: refs/heads/master@{#392761} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/1a26f1f5d6e4ecaa7f098302b17d8f87eb690362 Cr-Commit-Position: refs/heads/master@{#392761} |
