Steps: 1. Launch QtTestBrowser using ./Tools/Scripts/run-launcher --qt 2. Navigate to "Develop->Select Elements..." menu to enter the name of elements and press enter 3. The elements are highlighted on the page 4. Repeat step 2 with some other element name(valid/invalid element names) and press enter Expected outcome: Previously highlighted items should be restored while highlighting new elements if available. Actual outcome: Both the elements are hightlighted Proposed Solution: Provide a way to "Clear selection" in the "Develop" menu to clear the previous search highlight. Patch to follow.
Created attachment 183754 [details] Patch
Created attachment 183755 [details] Patch
Comment on attachment 183755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183755&action=review > Tools/QtTestBrowser/launcherwindow.cpp:76 > +struct ElementHighlight { How about calling this HighlightedElement? > Tools/QtTestBrowser/launcherwindow.cpp:78 > + QString m_styleValue; And this field m_oldStyle or m_previousStyle to emphasize the fact that this is the style from _before_ the element was highlighted. > Tools/QtTestBrowser/launcherwindow.cpp:825 > + ElementHighlight el = {e, e.styleProperty("background-color", QWebElement::InlineStyle)}; Space needed after { and before }. Does this even compile? I thought semicolon is needed after the styleProperty() call? > Tools/QtTestBrowser/launcherwindow.cpp:838 > +} You should clear the list here. > Tools/QtTestBrowser/launcherwindow.h:231 > + QList<ElementHighlight> m_previousHighlightElements; I suggest to call this m_highlightedElements to stress that this holds the list of currently highlighted elements, not the ones from the previous selection.
Created attachment 183758 [details] Patch
Comment on attachment 183755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183755&action=review Thank you for your comments. Uploaded a new patch. >> Tools/QtTestBrowser/launcherwindow.cpp:76 >> +struct ElementHighlight { > > How about calling this HighlightedElement? Done. >> Tools/QtTestBrowser/launcherwindow.cpp:78 >> + QString m_styleValue; > > And this field m_oldStyle or m_previousStyle to emphasize the fact that this is the style from _before_ the element was highlighted. Done. >> Tools/QtTestBrowser/launcherwindow.cpp:825 >> + ElementHighlight el = {e, e.styleProperty("background-color", QWebElement::InlineStyle)}; > > Space needed after { and before }. Does this even compile? I thought semicolon is needed after the styleProperty() call? Done. Yes it compiles :) >> Tools/QtTestBrowser/launcherwindow.cpp:838 >> +} > > You should clear the list here. Done. >> Tools/QtTestBrowser/launcherwindow.h:231 >> + QList<ElementHighlight> m_previousHighlightElements; > > I suggest to call this m_highlightedElements to stress that this holds the list of currently highlighted elements, not the ones from the previous selection. Done.
Comment on attachment 183758 [details] Patch Clearing flags on attachment: 183758 Committed r140323: <http://trac.webkit.org/changeset/140323>
All reviewed patches have been landed. Closing bug.