Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1824683002:
Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mouse… (Closed)
|
Created:
4 years, 9 months ago by nojvek Modified:
4 years, 7 months ago CC:
apavlov+blink_chromium.org, blink-reviews, blink-reviews-style_chromium.org, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman, sergeyv+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work
BUG=590477
Committed: https://crrev.com/00f8cd4924f855d92d9d05560a4fbe6e174fee04
Cr-Commit-Position: refs/heads/master@{#392402}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mouse… #
Total comments: 5
Patch Set 3 : iterate on review feedback #Patch Set 4 : show numeric precision modifier title hint on editing start #
Total comments: 5
Patch Set 5 : Fix review feedback + color shortcuts #Patch Set 6 : Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mouse… #
Messages
Total messages: 55 (11 generated)
Description was changed from ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 ========== to ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 ==========
nojvek@gmail.com changed reviewers: + alph@chromium.org, lushnikov@chromium.org, paulirish@chromium.org, pfeldman@chromium.org
https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2919: this._element.title = "Mousewheel or up/down keys. Ctrl: ±100, Shift: ±10, Alt: ±0.1"; this._element.title = WebInspector.UIString("Mousewheel or up/down keys. %s: ±100, Shift: ±10, Alt: ±0.1", , WebInspector.isMac() ? "Cmd" : "Ctrl"); https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right): https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:373: if (event.ctrlKey) WebInspector.KeyboardShortcut.eventHasCtrlOrMeta (to support Mac OS) https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:377: else if (event.shiftKey) Lets keep handlers in sorted order: 100, 10, 0.1
The CQ bit was checked by nojvek@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824683002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824683002/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
Now I know what the CQ checkbox does :P Sorry for the noise. On Mon, Mar 21, 2016 at 10:50 PM, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > No L-G-T-M from a valid reviewer yet. > CQ run can only be started by full committers or once the patch has > received an L-G-T-M from a full committer. > Even if an L-G-T-M may have been provided, it was from a non-committer, > _not_ a full super star committer. > See http://www.chromium.org/getting-involved/become-a-committer > Note that this has nothing to do with OWNERS files. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Now I know what the CQ checkbox does :P Sorry for the noise. On Mon, Mar 21, 2016 at 10:50 PM, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > No L-G-T-M from a valid reviewer yet. > CQ run can only be started by full committers or once the patch has > received an L-G-T-M from a full committer. > Even if an L-G-T-M may have been provided, it was from a non-committer, > _not_ a full super star committer. > See http://www.chromium.org/getting-involved/become-a-committer > Note that this has nothing to do with OWNERS files. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: if (!this._isEditingName && WebInspector.handleElementValueModifications(event, this._treeElement.valueElement, finishHandler.bind(this), this._isValueSuggestion.bind(this), customNumberHandler.bind(this))){ style: space before "{" https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2918: if(!this._element.title) why do we setup title so late? we should probably do it on editing start. also, this._element is a private member of TextPrompt; let's create a TextPrompt.setTitle public method instead of accessing private members here. https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right): https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:304: // When shift is pressed while spinning mousewheel, delta comes as wheelDeltaX style: comments should end with "."
Fixed lushnikov's feedback. I didn't see a hook for onEditStart so I created one for onEditStart, onEditDone. Also TextPrompt has title, and setTitle functions so we don't access this._element private var. P.S Have you considered using typescript? https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right): https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:373: if (event.ctrlKey) On 2016/03/21 18:18:30, pfeldman wrote: > WebInspector.KeyboardShortcut.eventHasCtrlOrMeta (to support Mac OS) Done. https://codereview.chromium.org/1824683002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:377: else if (event.shiftKey) On 2016/03/21 18:18:30, pfeldman wrote: > Lets keep handlers in sorted order: 100, 10, 0.1 Done. https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: if (!this._isEditingName && WebInspector.handleElementValueModifications(event, this._treeElement.valueElement, finishHandler.bind(this), this._isValueSuggestion.bind(this), customNumberHandler.bind(this))){ On 2016/03/22 17:21:42, lushnikov wrote: > style: space before "{" Done. https://codereview.chromium.org/1824683002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2918: if(!this._element.title) On 2016/03/22 17:21:42, lushnikov wrote: > why do we setup title so late? we should probably do it on editing start. > > also, this._element is a private member of TextPrompt; let's create a > TextPrompt.setTitle public method instead of accessing private members here. Done.
Thank you nojvek for getting your hands dirty with this! The code looks good, but i think it could be simplified a little bit. https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: if (!this._isEditingName && WebInspector.handleElementValueModifications(event, this._treeElement.valueElement, finishHandler.bind(this), this._isValueSuggestion.bind(this), customNumberHandler.bind(this))) { style: omit braces https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2927: onEditStart: function() I think we can simplify this even further. Let's not add the onEditStart and onEditDone hooks to keep the TextPrompt interface minimal. Instead, the contents of this method can move into the end of WI.StylesSidebarPane.CSSPropertyPrompt constructor. In this case, we would not be able to call both this.title() and this.text() methods in the if-condition, as the prompt is not attached yet. However, this is not an issue: - this.text() could be just substituted with manual fetching of text: var text = isEditingName ? treeElement.nameElement.textContent : treeElement.valueElement.textContent; - this.title() should not depend on the prompt attachment. (see comments in TextPrompt.js) https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2929: // if a css value is being edited that has a numeric substring, hint that precision modifier shortcuts are available style: comments should start with capital letter and finish with dot. https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:197: return this._element.title; In order to make methods title() and setTitle() independent of prompt attachment (indeed, why should they depend on that?), let's add a private member this._title and return it here https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:205: this._element.title = title; I think it would be better to use this._proxyElement instead of this._element: the this._element comes from the outside, whereas the proxyElement is under our full control. This method will translate into something like this: this._title = title; if (this._proxyElement) this._proxyElement = ... Note: you might also need to update the proxyElements' title at the moment of its creation in the _attachInternal.
Will take another stab at it. I only understand bits and pieces of devtools ui framework What does the proxy element do? Thanks for looking into this. On Friday, March 25, 2016, <lushnikov@chromium.org> wrote: > Thank you nojvek for getting your hands dirty with this! > > The code looks good, but i think it could be simplified a little bit. > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > File > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js > (right): > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: > if (!this._isEditingName && > WebInspector.handleElementValueModifications(event, > this._treeElement.valueElement, finishHandler.bind(this), > this._isValueSuggestion.bind(this), customNumberHandler.bind(this))) { > style: omit braces > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2927: > onEditStart: function() > I think we can simplify this even further. > > Let's not add the onEditStart and onEditDone hooks to keep the > TextPrompt interface minimal. > Instead, the contents of this method can move into the end of > WI.StylesSidebarPane.CSSPropertyPrompt constructor. > > In this case, we would not be able to call both this.title() and > this.text() methods in the if-condition, as the prompt is not attached > yet. However, this is not an issue: > - this.text() could be just substituted with manual fetching of text: > > var text = isEditingName ? treeElement.nameElement.textContent : > treeElement.valueElement.textContent; > > - this.title() should not depend on the prompt attachment. (see comments > in TextPrompt.js) > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2929: > // if a css value is being edited that has a numeric substring, hint > that precision modifier shortcuts are available > style: comments should start with capital letter and finish with dot. > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js > (right): > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:197: > return this._element.title; > In order to make methods title() and setTitle() independent of prompt > attachment (indeed, why should they depend on that?), let's add a > private member this._title and return it here > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:205: > this._element.title = title; > I think it would be better to use this._proxyElement instead of > this._element: > the this._element comes from the outside, whereas the proxyElement is > under our full control. > > This method will translate into something like this: > > this._title = title; > if (this._proxyElement) > this._proxyElement = ... > > > Note: you might also need to update the proxyElements' title at the > moment of its creation in the > _attachInternal. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Will take another stab at it. I only understand bits and pieces of devtools ui framework What does the proxy element do? Thanks for looking into this. On Friday, March 25, 2016, <lushnikov@chromium.org> wrote: > Thank you nojvek for getting your hands dirty with this! > > The code looks good, but i think it could be simplified a little bit. > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > File > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js > (right): > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: > if (!this._isEditingName && > WebInspector.handleElementValueModifications(event, > this._treeElement.valueElement, finishHandler.bind(this), > this._isValueSuggestion.bind(this), customNumberHandler.bind(this))) { > style: omit braces > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2927: > onEditStart: function() > I think we can simplify this even further. > > Let's not add the onEditStart and onEditDone hooks to keep the > TextPrompt interface minimal. > Instead, the contents of this method can move into the end of > WI.StylesSidebarPane.CSSPropertyPrompt constructor. > > In this case, we would not be able to call both this.title() and > this.text() methods in the if-condition, as the prompt is not attached > yet. However, this is not an issue: > - this.text() could be just substituted with manual fetching of text: > > var text = isEditingName ? treeElement.nameElement.textContent : > treeElement.valueElement.textContent; > > - this.title() should not depend on the prompt attachment. (see comments > in TextPrompt.js) > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2929: > // if a css value is being edited that has a numeric substring, hint > that precision modifier shortcuts are available > style: comments should start with capital letter and finish with dot. > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js > (right): > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:197: > return this._element.title; > In order to make methods title() and setTitle() independent of prompt > attachment (indeed, why should they depend on that?), let's add a > private member this._title and return it here > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:205: > this._element.title = title; > I think it would be better to use this._proxyElement instead of > this._element: > the this._element comes from the outside, whereas the proxyElement is > under our full control. > > This method will translate into something like this: > > this._title = title; > if (this._proxyElement) > this._proxyElement = ... > > > Note: you might also need to update the proxyElements' title at the > moment of its creation in the > _attachInternal. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Fixed feedback. On 2016/03/25 21:27:16, nojvek wrote: > Will take another stab at it. I only understand bits and pieces of > devtools ui framework > > What does the proxy element do? > > Thanks for looking into this. > > On Friday, March 25, 2016, <mailto:lushnikov@chromium.org> wrote: > > > Thank you nojvek for getting your hands dirty with this! > > > > The code looks good, but i think it could be simplified a little bit. > > > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > File > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js > > (right): > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > > > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2917: > > if (!this._isEditingName && > > WebInspector.handleElementValueModifications(event, > > this._treeElement.valueElement, finishHandler.bind(this), > > this._isValueSuggestion.bind(this), customNumberHandler.bind(this))) { > > style: omit braces > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > > > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2927: > > onEditStart: function() > > I think we can simplify this even further. > > > > Let's not add the onEditStart and onEditDone hooks to keep the > > TextPrompt interface minimal. > > Instead, the contents of this method can move into the end of > > WI.StylesSidebarPane.CSSPropertyPrompt constructor. > > > > In this case, we would not be able to call both this.title() and > > this.text() methods in the if-condition, as the prompt is not attached > > yet. However, this is not an issue: > > - this.text() could be just substituted with manual fetching of text: > > > > var text = isEditingName ? treeElement.nameElement.textContent : > > treeElement.valueElement.textContent; > > > > - this.title() should not depend on the prompt attachment. (see comments > > in TextPrompt.js) > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > > > > third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2929: > > // if a css value is being edited that has a numeric substring, hint > > that precision modifier shortcuts are available > > style: comments should start with capital letter and finish with dot. > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js > > (right): > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:197: > > return this._element.title; > > In order to make methods title() and setTitle() independent of prompt > > attachment (indeed, why should they depend on that?), let's add a > > private member this._title and return it here > > > > > > > https://codereview.chromium.org/1824683002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:205: > > this._element.title = title; > > I think it would be better to use this._proxyElement instead of > > this._element: > > the this._element comes from the outside, whereas the proxyElement is > > under our full control. > > > > This method will translate into something like this: > > > > this._title = title; > > if (this._proxyElement) > > this._proxyElement = ... > > > > > > Note: you might also need to update the proxyElements' title at the > > moment of its creation in the > > _attachInternal. > > > > https://codereview.chromium.org/1824683002/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
Nice! Thank you, LGTM
The CQ bit was checked by nojvek@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824683002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824683002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
This means some tests need to be fixed as well right? How do I go about running them locally? On Mon, Mar 28, 2016 at 2:27 PM, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > Try jobs failed on following builders: > linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, > > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_... > ) > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
This means some tests need to be fixed as well right? How do I go about running them locally? On Mon, Mar 28, 2016 at 2:27 PM, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > Try jobs failed on following builders: > linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, > > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_... > ) > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
This will require checking out chromium and building it from sources. The downside, the checkout is huge. Will you be able to do that? The instructions to run tests: https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Running-L...
I don't know whether my 4 year old poor macbook air can handle that. I can give it a shot. I tried going through the failures but didn't see any stack traces where the exact error was happening and what I needed to fix. I tried clicking on the cache logs link but I get this. https://storage.cloud.google.com/?arg=chrome-build-logs-private/logs/tryserve... The Google Storage Manager is deprecated, please use the Cloud Storage Viewer <https://console.developers.google.com/storage> in the Cloud Console. On Mon, Mar 28, 2016 at 6:11 PM, <lushnikov@chromium.org> wrote: > This will require checking out chromium and building it from sources. The > downside, the checkout is huge. Will you be able to do that? > > The instructions to run tests: > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Running-L... > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
I don't know whether my 4 year old poor macbook air can handle that. I can give it a shot. I tried going through the failures but didn't see any stack traces where the exact error was happening and what I needed to fix. I tried clicking on the cache logs link but I get this. https://storage.cloud.google.com/?arg=chrome-build-logs-private/logs/tryserve... The Google Storage Manager is deprecated, please use the Cloud Storage Viewer <https://console.developers.google.com/storage> in the Cloud Console. On Mon, Mar 28, 2016 at 6:11 PM, <lushnikov@chromium.org> wrote: > This will require checking out chromium and building it from sources. The > downside, the checkout is huge. Will you be able to do that? > > The instructions to run tests: > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Running-L... > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/03/29 01:39:16, nojvek wrote: > I don't know whether my 4 year old poor macbook air can handle that. I can > give it a shot. If you fail, I can help you with tests. > > I tried going through the failures but didn't see any stack traces where > the exact error was happening and what I needed to fix. The DevTools tests are a part of layout tests; they dumb some values into the output. As far as the values differ from the expected, the test fails. To get the failed tests, click on "Layout_test_results" link in the section #23. Here's the screenshot with the link: http://i.imgur.com/1GGzDM8.png > > I tried clicking on the cache logs link but I get this. > > https://storage.cloud.google.com/?arg=chrome-build-logs-private/logs/tryserve... > > The Google Storage Manager is deprecated, please use the Cloud Storage > Viewer <https://console.developers.google.com/storage> in the Cloud Console. > > > > > On Mon, Mar 28, 2016 at 6:11 PM, <mailto:lushnikov@chromium.org> wrote: > > > This will require checking out chromium and building it from sources. The > > downside, the checkout is huge. Will you be able to do that? > > > > The instructions to run tests: > > > > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Running-L... > > > > https://codereview.chromium.org/1824683002/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
This never landed. I'm restarting the try jobs so we can work from the errors.
Thanks Paul. This week I'm not on fire so I can probably spend a couple of hours to get this through. Regards. On Monday, April 25, 2016, <paulirish@chromium.org> wrote: > This never landed. > > I'm restarting the try jobs so we can work from the errors. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Thanks Paul. This week I'm not on fire so I can probably spend a couple of hours to get this through. Regards. On Monday, April 25, 2016, <paulirish@chromium.org> wrote: > This never landed. > > I'm restarting the try jobs so we can work from the errors. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Results are in: https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r... I recommend the "pretty diff" links in there.
So where does the source code for the tests reside? And any way to run the suite locally? On Monday, April 25, 2016, <paulirish@chromium.org> wrote: > Results are in: > > https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r... > > I recommend the "pretty diff" links in there. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
So where does the source code for the tests reside? And any way to run the suite locally? On Monday, April 25, 2016, <paulirish@chromium.org> wrote: > Results are in: > > https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r... > > I recommend the "pretty diff" links in there. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/04/26 at 03:42:49, nojvek wrote: > So where does the source code for the tests reside? https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > And any way to run the suite locally? https://www.chromium.org/developers/testing/webkit-layout-tests https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... It requires compiling content shell, which is somewhat annoying. An alternative is to take the page (https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st...) and its inspector-test dependencies, and run them via Snippets. That should at least get you the results.
any help needed with tests? It would be great to have this finally landed.
Oh I didn't see Paul's reply. I'll work on it tonight. My bad, sorry. On Wed, May 4, 2016 at 4:09 PM, <lushnikov@chromium.org> wrote: > any help needed with tests? It would be great to have this finally landed. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Oh I didn't see Paul's reply. I'll work on it tonight. My bad, sorry. On Wed, May 4, 2016 at 4:09 PM, <lushnikov@chromium.org> wrote: > any help needed with tests? It would be great to have this finally landed. > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
So I bashed my head trying to get content_shell to compile. How does the ninja command work? It complains about build.ninja file missing. Could I get the binary prebuilt for mac/windows? I tried to also alternatively run snippets. Here's what I did. Open http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... (devtools frontend on google homepage) Inject http/tests/inspector/inspector-test.js Inject http/tests/inspector/elements-test.js Inject the test script Manually call runTests() - Nothing happened call test() VM603:3 Uncaught ReferenceError: InspectorTest is not defined(…) Am I missing another dependancy? Regards. On Mon, Apr 25, 2016 at 9:03 PM, <paulirish@chromium.org> wrote: > On 2016/04/26 at 03:42:49, nojvek wrote: > > So where does the source code for the tests reside? > > > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > > > And any way to run the suite locally? > > https://www.chromium.org/developers/testing/webkit-layout-tests > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > > It requires compiling content shell, which is somewhat annoying. > > An alternative is to take the page > ( > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > ) > and its inspector-test dependencies, and run them via Snippets. > That should at least get you the results. > > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
So I bashed my head trying to get content_shell to compile. How does the ninja command work? It complains about build.ninja file missing. Could I get the binary prebuilt for mac/windows? I tried to also alternatively run snippets. Here's what I did. Open http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... (devtools frontend on google homepage) Inject http/tests/inspector/inspector-test.js Inject http/tests/inspector/elements-test.js Inject the test script Manually call runTests() - Nothing happened call test() VM603:3 Uncaught ReferenceError: InspectorTest is not defined(…) Am I missing another dependancy? Regards. On Mon, Apr 25, 2016 at 9:03 PM, <paulirish@chromium.org> wrote: > On 2016/04/26 at 03:42:49, nojvek wrote: > > So where does the source code for the tests reside? > > > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > > > And any way to run the suite locally? > > https://www.chromium.org/developers/testing/webkit-layout-tests > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > > It requires compiling content shell, which is somewhat annoying. > > An alternative is to take the page > ( > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > ) > and its inspector-test dependencies, and run them via Snippets. > That should at least get you the results. > > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Tried to give another shot to get contentshell to compile. Searched entire project for .ninja files. I couldn't find any. I'm guessing this are autogenerated by some tool? I also searched for any .xcodeproj files, I couldn't find any. Building somehow links to this project https://bugs.chromium.org/p/chromium/adminIntro but it seems to be abandoned. I see some references to gyp, but pip install gyp gives me: ➜ chromium pip install gyp Collecting gyp Could not find a version that satisfies the requirement gyp (from versions: ) No matching distribution found for gyp Is there someone I can IM? Regards. On Wed, May 4, 2016 at 11:27 PM, Noj Vek <nojvek@gmail.com> wrote: > So I bashed my head trying to get content_shell to compile. > > How does the ninja command work? It complains about build.ninja file > missing. > > Could I get the binary prebuilt for mac/windows? > > I tried to also alternatively run snippets. Here's what I did. > > Open > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > (devtools frontend on google homepage) > > Inject http/tests/inspector/inspector-test.js > Inject http/tests/inspector/elements-test.js > > Inject the test script > > Manually call runTests() - Nothing happened > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > defined(…) > > Am I missing another dependancy? > > Regards. > > > On Mon, Apr 25, 2016 at 9:03 PM, <paulirish@chromium.org> wrote: > >> On 2016/04/26 at 03:42:49, nojvek wrote: >> > So where does the source code for the tests reside? >> >> >> https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... >> >> > And any way to run the suite locally? >> >> https://www.chromium.org/developers/testing/webkit-layout-tests >> >> https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... >> >> It requires compiling content shell, which is somewhat annoying. >> >> An alternative is to take the page >> ( >> https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... >> ) >> and its inspector-test dependencies, and run them via Snippets. >> That should at least get you the results. >> >> >> https://codereview.chromium.org/1824683002/ >> > > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Tried to give another shot to get contentshell to compile. Searched entire project for .ninja files. I couldn't find any. I'm guessing this are autogenerated by some tool? I also searched for any .xcodeproj files, I couldn't find any. Building somehow links to this project https://bugs.chromium.org/p/chromium/adminIntro but it seems to be abandoned. I see some references to gyp, but pip install gyp gives me: ➜ chromium pip install gyp Collecting gyp Could not find a version that satisfies the requirement gyp (from versions: ) No matching distribution found for gyp Is there someone I can IM? Regards. On Wed, May 4, 2016 at 11:27 PM, Noj Vek <nojvek@gmail.com> wrote: > So I bashed my head trying to get content_shell to compile. > > How does the ninja command work? It complains about build.ninja file > missing. > > Could I get the binary prebuilt for mac/windows? > > I tried to also alternatively run snippets. Here's what I did. > > Open > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > (devtools frontend on google homepage) > > Inject http/tests/inspector/inspector-test.js > Inject http/tests/inspector/elements-test.js > > Inject the test script > > Manually call runTests() - Nothing happened > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > defined(…) > > Am I missing another dependancy? > > Regards. > > > On Mon, Apr 25, 2016 at 9:03 PM, <paulirish@chromium.org> wrote: > >> On 2016/04/26 at 03:42:49, nojvek wrote: >> > So where does the source code for the tests reside? >> >> >> https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... >> >> > And any way to run the suite locally? >> >> https://www.chromium.org/developers/testing/webkit-layout-tests >> >> https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... >> >> It requires compiling content shell, which is somewhat annoying. >> >> An alternative is to take the page >> ( >> https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... >> ) >> and its inspector-test dependencies, and run them via Snippets. >> That should at least get you the results. >> >> >> https://codereview.chromium.org/1824683002/ >> > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/05/06 06:05:20, nojvek wrote: > Tried to give another shot to get contentshell to compile. > > Searched entire project for .ninja files. I couldn't find any. I'm guessing > this are autogenerated by some tool? > > I also searched for any .xcodeproj files, I couldn't find any. > > Building somehow links to this project > https://bugs.chromium.org/p/chromium/adminIntro but it seems to be > abandoned. > > I see some references to gyp, but pip install gyp gives me: > > ➜ chromium pip install gyp > > Collecting gyp > > Could not find a version that satisfies the requirement gyp (from > versions: ) > > No matching distribution found for gyp > > Is there someone I can IM? > > Regards. > > > > > On Wed, May 4, 2016 at 11:27 PM, Noj Vek <mailto:nojvek@gmail.com> wrote: > > > So I bashed my head trying to get content_shell to compile. > > > > How does the ninja command work? It complains about build.ninja file > > missing. > > > > Could I get the binary prebuilt for mac/windows? > > > > I tried to also alternatively run snippets. Here's what I did. > > > > Open > > > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > > (devtools frontend on google homepage) > > > > Inject http/tests/inspector/inspector-test.js > > Inject http/tests/inspector/elements-test.js > > > > Inject the test script > > > > Manually call runTests() - Nothing happened > > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > > defined(…) > > > > Am I missing another dependancy? > > > > Regards. > > > > > > On Mon, Apr 25, 2016 at 9:03 PM, <mailto:paulirish@chromium.org> wrote: > > > >> On 2016/04/26 at 03:42:49, nojvek wrote: > >> > So where does the source code for the tests reside? > >> > >> > >> > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > >> > >> > And any way to run the suite locally? > >> > >> https://www.chromium.org/developers/testing/webkit-layout-tests > >> > >> > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > >> > >> It requires compiling content shell, which is somewhat annoying. > >> > >> An alternative is to take the page > >> ( > >> > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > >> ) > >> and its inspector-test dependencies, and run them via Snippets. > >> That should at least get you the results. > >> > >> > >> https://codereview.chromium.org/1824683002/ > >> > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. Hi Nojvek, You can IM me on hangouts: lushnikov@google.com, I would be happy to assist
Lushnikov, Paul offered a tip that I should ensure I have full chromium checkout and the run client sync and then attempt to build. But if I get stuck, I'll IM you. Thanks. On Friday, May 6, 2016, <lushnikov@chromium.org> wrote: > On 2016/05/06 06:05:20, nojvek wrote: > > Tried to give another shot to get contentshell to compile. > > > > Searched entire project for .ninja files. I couldn't find any. I'm > guessing > > this are autogenerated by some tool? > > > > I also searched for any .xcodeproj files, I couldn't find any. > > > > Building somehow links to this project > > https://bugs.chromium.org/p/chromium/adminIntro but it seems to be > > abandoned. > > > > I see some references to gyp, but pip install gyp gives me: > > > > ➜ chromium pip install gyp > > > > Collecting gyp > > > > Could not find a version that satisfies the requirement gyp (from > > versions: ) > > > > No matching distribution found for gyp > > > > Is there someone I can IM? > > > > Regards. > > > > > > > > > > On Wed, May 4, 2016 at 11:27 PM, Noj Vek <mailto:nojvek@gmail.com > <javascript:_e(%7B%7D,'cvml','nojvek@gmail.com');>> wrote: > > > > > So I bashed my head trying to get content_shell to compile. > > > > > > How does the ninja command work? It complains about build.ninja file > > > missing. > > > > > > Could I get the binary prebuilt for mac/windows? > > > > > > I tried to also alternatively run snippets. Here's what I did. > > > > > > Open > > > > > > > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > > > (devtools frontend on google homepage) > > > > > > Inject http/tests/inspector/inspector-test.js > > > Inject http/tests/inspector/elements-test.js > > > > > > Inject the test script > > > > > > Manually call runTests() - Nothing happened > > > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > > > defined(…) > > > > > > Am I missing another dependancy? > > > > > > Regards. > > > > > > > > > On Mon, Apr 25, 2016 at 9:03 PM, <mailto:paulirish@chromium.org > <javascript:_e(%7B%7D,'cvml','paulirish@chromium.org');>> wrote: > > > > > >> On 2016/04/26 at 03:42:49, nojvek wrote: > > >> > So where does the source code for the tests reside? > > >> > > >> > > >> > > > > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > > >> > > >> > And any way to run the suite locally? > > >> > > >> https://www.chromium.org/developers/testing/webkit-layout-tests > > >> > > >> > > > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > > >> > > >> It requires compiling content shell, which is somewhat annoying. > > >> > > >> An alternative is to take the page > > >> ( > > >> > > > > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > > >> ) > > >> and its inspector-test dependencies, and run them via Snippets. > > >> That should at least get you the results. > > >> > > >> > > >> https://codereview.chromium.org/1824683002/ > > >> > > > > > > > > > > -- > > You received this message because you are subscribed to the Google Groups > > "Chromium-reviews" group. > > To unsubscribe from this group and stop receiving emails from it, send an > email > > to mailto:chromium-reviews+unsubscribe@chromium.org > <javascript:_e(%7B%7D,'cvml','chromium-reviews%2Bunsubscribe@chromium.org');> > . > > Hi Nojvek, > > You can IM me on hangouts: lushnikov@google.com > <javascript:_e(%7B%7D,'cvml','lushnikov@google.com');>, I would be happy > to assist > > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Lushnikov, Paul offered a tip that I should ensure I have full chromium checkout and the run client sync and then attempt to build. But if I get stuck, I'll IM you. Thanks. On Friday, May 6, 2016, <lushnikov@chromium.org> wrote: > On 2016/05/06 06:05:20, nojvek wrote: > > Tried to give another shot to get contentshell to compile. > > > > Searched entire project for .ninja files. I couldn't find any. I'm > guessing > > this are autogenerated by some tool? > > > > I also searched for any .xcodeproj files, I couldn't find any. > > > > Building somehow links to this project > > https://bugs.chromium.org/p/chromium/adminIntro but it seems to be > > abandoned. > > > > I see some references to gyp, but pip install gyp gives me: > > > > ➜ chromium pip install gyp > > > > Collecting gyp > > > > Could not find a version that satisfies the requirement gyp (from > > versions: ) > > > > No matching distribution found for gyp > > > > Is there someone I can IM? > > > > Regards. > > > > > > > > > > On Wed, May 4, 2016 at 11:27 PM, Noj Vek <mailto:nojvek@gmail.com > <javascript:_e(%7B%7D,'cvml','nojvek@gmail.com');>> wrote: > > > > > So I bashed my head trying to get content_shell to compile. > > > > > > How does the ninja command work? It complains about build.ninja file > > > missing. > > > > > > Could I get the binary prebuilt for mac/windows? > > > > > > I tried to also alternatively run snippets. Here's what I did. > > > > > > Open > > > > > > > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > > > (devtools frontend on google homepage) > > > > > > Inject http/tests/inspector/inspector-test.js > > > Inject http/tests/inspector/elements-test.js > > > > > > Inject the test script > > > > > > Manually call runTests() - Nothing happened > > > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > > > defined(…) > > > > > > Am I missing another dependancy? > > > > > > Regards. > > > > > > > > > On Mon, Apr 25, 2016 at 9:03 PM, <mailto:paulirish@chromium.org > <javascript:_e(%7B%7D,'cvml','paulirish@chromium.org');>> wrote: > > > > > >> On 2016/04/26 at 03:42:49, nojvek wrote: > > >> > So where does the source code for the tests reside? > > >> > > >> > > >> > > > > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > > >> > > >> > And any way to run the suite locally? > > >> > > >> https://www.chromium.org/developers/testing/webkit-layout-tests > > >> > > >> > > > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > > >> > > >> It requires compiling content shell, which is somewhat annoying. > > >> > > >> An alternative is to take the page > > >> ( > > >> > > > > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > > >> ) > > >> and its inspector-test dependencies, and run them via Snippets. > > >> That should at least get you the results. > > >> > > >> > > >> https://codereview.chromium.org/1824683002/ > > >> > > > > > > > > > > -- > > You received this message because you are subscribed to the Google Groups > > "Chromium-reviews" group. > > To unsubscribe from this group and stop receiving emails from it, send an > email > > to mailto:chromium-reviews+unsubscribe@chromium.org > <javascript:_e(%7B%7D,'cvml','chromium-reviews%2Bunsubscribe@chromium.org');> > . > > Hi Nojvek, > > You can IM me on hangouts: lushnikov@google.com > <javascript:_e(%7B%7D,'cvml','lushnikov@google.com');>, I would be happy > to assist > > > https://codereview.chromium.org/1824683002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by nojvek@gmail.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824683002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824683002/100001
Hi Folks, Thank you Paul and Lushnikov for the help. Building content_shell took most of the saturday. Apart from macbook air running out of disk space and the fan going nuts, it went pretty smooth. Fixed the test issues and uploaded a new patch. I ran all the inspector tests to ensured they passed. Enjoy your weekend. Regards. On 2016/05/06 18:03:50, nojvek wrote: > Lushnikov, > > Paul offered a tip that I should ensure I have full chromium checkout and > the run client sync and then attempt to build. > > But if I get stuck, I'll IM you. > > Thanks. > > On Friday, May 6, 2016, <mailto:lushnikov@chromium.org> wrote: > > > On 2016/05/06 06:05:20, nojvek wrote: > > > Tried to give another shot to get contentshell to compile. > > > > > > Searched entire project for .ninja files. I couldn't find any. I'm > > guessing > > > this are autogenerated by some tool? > > > > > > I also searched for any .xcodeproj files, I couldn't find any. > > > > > > Building somehow links to this project > > > https://bugs.chromium.org/p/chromium/adminIntro but it seems to be > > > abandoned. > > > > > > I see some references to gyp, but pip install gyp gives me: > > > > > > ➜ chromium pip install gyp > > > > > > Collecting gyp > > > > > > Could not find a version that satisfies the requirement gyp (from > > > versions: ) > > > > > > No matching distribution found for gyp > > > > > > Is there someone I can IM? > > > > > > Regards. > > > > > > > > > > > > > > > On Wed, May 4, 2016 at 11:27 PM, Noj Vek <mailto:nojvek@gmail.com > > <javascript:_e(%7B%7D,'cvml','nojvek@gmail.com');>> wrote: > > > > > > > So I bashed my head trying to get content_shell to compile. > > > > > > > > How does the ninja command work? It complains about build.ninja file > > > > missing. > > > > > > > > Could I get the binary prebuilt for mac/windows? > > > > > > > > I tried to also alternatively run snippets. Here's what I did. > > > > > > > > Open > > > > > > > > > > > > http://localhost:8080/front_end/inspector.html?ws=localhost:9999/devtools/pag... > > > > (devtools frontend on google homepage) > > > > > > > > Inject http/tests/inspector/inspector-test.js > > > > Inject http/tests/inspector/elements-test.js > > > > > > > > Inject the test script > > > > > > > > Manually call runTests() - Nothing happened > > > > call test() VM603:3 Uncaught ReferenceError: InspectorTest is not > > > > defined(…) > > > > > > > > Am I missing another dependancy? > > > > > > > > Regards. > > > > > > > > > > > > On Mon, Apr 25, 2016 at 9:03 PM, <mailto:paulirish@chromium.org > > <javascript:_e(%7B%7D,'cvml','paulirish@chromium.org');>> wrote: > > > > > > > >> On 2016/04/26 at 03:42:49, nojvek wrote: > > > >> > So where does the source code for the tests reside? > > > >> > > > >> > > > >> > > > > > > > > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La... > > > >> > > > >> > And any way to run the suite locally? > > > >> > > > >> https://www.chromium.org/developers/testing/webkit-layout-tests > > > >> > > > >> > > > > > > > > https://www.chromium.org/developers/testing/webkit-layout-tests#TOC-Debugging... > > > >> > > > >> It requires compiling content shell, which is somewhat annoying. > > > >> > > > >> An alternative is to take the page > > > >> ( > > > >> > > > > > > > > https://src.chromium.org/viewvc/blink/trunk/LayoutTests/inspector/elements/st... > > > >> ) > > > >> and its inspector-test dependencies, and run them via Snippets. > > > >> That should at least get you the results. > > > >> > > > >> > > > >> https://codereview.chromium.org/1824683002/ > > > >> > > > > > > > > > > > > > > -- > > > You received this message because you are subscribed to the Google Groups > > > "Chromium-reviews" group. > > > To unsubscribe from this group and stop receiving emails from it, send an > > email > > > to mailto:chromium-reviews+unsubscribe@chromium.org > > <javascript:_e(%7B%7D,'cvml','chromium-reviews%2Bunsubscribe@chromium.org');> > > . > > > > Hi Nojvek, > > > > You can IM me on hangouts: mailto:lushnikov@google.com > > <javascript:_e(%7B%7D,'cvml','lushnikov@google.com');>, I would be happy > > to assist > > > > > > https://codereview.chromium.org/1824683002/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Great to hear you did that! Thank you! One more LGTM!
The CQ bit was checked by lushnikov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824683002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824683002/100001
Message was sent while issue was closed.
Description was changed from ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 ========== to ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 ========== to ========== Fixes 590477: Style pane numeric property change modifier shortcuts are undiscoverable. Shift mousewheel doesn't work BUG=590477 Committed: https://crrev.com/00f8cd4924f855d92d9d05560a4fbe6e174fee04 Cr-Commit-Position: refs/heads/master@{#392402} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/00f8cd4924f855d92d9d05560a4fbe6e174fee04 Cr-Commit-Position: refs/heads/master@{#392402} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
