Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1862243004:
Remove the flag for search text fake omnibox from function that is used elsewhere to localised point (Closed)
|
Created:
4 years, 8 months ago by mcwilliams Modified:
4 years, 8 months ago CC:
chromium-reviews, mcwilliams+watch_chromium.org, dgn+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@always-show-snippets Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove the flag for search text fake omnibox from function that is used elsewhere to localised point
This is because the: isInSingleUrlBarMode function is used in more than
one place which is where I added the flag to. This will affect the
locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002
BUG=600759
Committed: https://crrev.com/ffc42198e6c142eabf3aa3b6a2bbaf56922a62bf
Cr-Commit-Position: refs/heads/master@{#386021}
Patch Set 1 #Patch Set 2 : #
Messages
Total messages: 28 (11 generated)
mcwilliams@chromium.org changed reviewers: + maybelle@chromium.org
mcwilliams@chromium.org changed reviewers: + bauerb@chromium.org
Could you edit the commit message to indicate the issue you're fixing by doing this? It's not obvious what's wrong and why this change is needed.
On 2016/04/07 14:19:00, May wrote: > Could you edit the commit message to indicate the issue you're fixing by doing > this? It's not obvious what's wrong and why this change is needed. As discussed this is not for the same bug :)
lgtm
On 2016/04/07 14:53:04, mcwilliams wrote: > On 2016/04/07 14:19:00, May wrote: > > Could you edit the commit message to indicate the issue you're fixing by doing > > this? It's not obvious what's wrong and why this change is needed. > > As discussed this is not for the same bug :) Could you still add a bug reference for this? I also don't quite understand what this does and why 😃
Description was changed from ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point BUG= ========== to ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point BUG=600759 ==========
On 2016/04/07 16:26:40, Bernhard Bauer wrote: > On 2016/04/07 14:53:04, mcwilliams wrote: > > On 2016/04/07 14:19:00, May wrote: > > > Could you edit the commit message to indicate the issue you're fixing by > doing > > > this? It's not obvious what's wrong and why this change is needed. > > > > As discussed this is not for the same bug :) > > Could you still add a bug reference for this? I also don't quite understand what > this does and why 😃 Done. This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl.
The CQ bit was checked by mcwilliams@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1862243004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1862243004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
On 2016/04/07 16:30:58, mcwilliams wrote: > On 2016/04/07 16:26:40, Bernhard Bauer wrote: > > On 2016/04/07 14:53:04, mcwilliams wrote: > > > On 2016/04/07 14:19:00, May wrote: > > > > Could you edit the commit message to indicate the issue you're fixing by > > doing > > > > this? It's not obvious what's wrong and why this change is needed. > > > > > > As discussed this is not for the same bug :) > > > > Could you still add a bug reference for this? I also don't quite understand > what > > this does and why 😃 > > Done. This is because the: isInSingleUrlBarMode function is used in more than > one place which is where I added the flag to. This will affect the > locationUrlBar if left as is which was no my intention of the original cl. OK, thanks for the explanation. Could you also add that to the CL description, and maybe mention that it's a followup to https://codereview.chromium.org/1857303002? Thanks!
Description was changed from ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point BUG=600759 ========== to ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002 BUG=600759 ==========
The CQ bit was checked by mcwilliams@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1862243004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1862243004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
LGTM (FTR, Rietveld doesn't send out emails when the description changes, so you have to manually reply, which is why I didn't see this before).
On 2016/04/08 08:33:44, Bernhard Bauer wrote: > LGTM > > (FTR, Rietveld doesn't send out emails when the description changes, so you have > to manually reply, which is why I didn't see this before). Thanks I did not realise that - very different to critique
The CQ bit was checked by mcwilliams@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1862243004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1862243004/20001
Message was sent while issue was closed.
Description was changed from ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002 BUG=600759 ========== to ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002 BUG=600759 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002 BUG=600759 ========== to ========== Remove the flag for search text fake omnibox from function that is used elsewhere to localised point This is because the: isInSingleUrlBarMode function is used in more than one place which is where I added the flag to. This will affect the locationUrlBar if left as is which was no my intention of the original cl: https://codereview.chromium.org/1857303002 BUG=600759 Committed: https://crrev.com/ffc42198e6c142eabf3aa3b6a2bbaf56922a62bf Cr-Commit-Position: refs/heads/master@{#386021} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ffc42198e6c142eabf3aa3b6a2bbaf56922a62bf Cr-Commit-Position: refs/heads/master@{#386021} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
