Bug 29961

Summary: Position of "Close" button in docked mode doesn't match the position of window "Close" button in detached mode
Product: WebKit Reporter: Mikhail Naganov <mnaganov>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: pfeldman, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
Attachments:
Description Flags
propsed change
timothy: review-
[IMAGE] close to the right (don't pay attention to shifted search field - i have an old and strange WebKit build)
none
Screenshot with Safari 4.0.3 and WebKit 48959
none
update patch
timothy: review-
removed substring matching timothy: review+, timothy: commit-queue-

Mikhail Naganov
Reported 2009-10-01 07:36:37 PDT
Currently on Windows "Close" button is on the left side in docked mode, but the "Close" button of a window in detached mode is on the right side. Let's place "Close" button in docked mode on the same side as the window "Close" button in detached mode.
Attachments
propsed change (2.94 KB, patch)
2009-10-01 07:44 PDT, Mikhail Naganov
timothy: review-
[IMAGE] close to the right (don't pay attention to shifted search field - i have an old and strange WebKit build) (222.40 KB, image/png)
2009-10-01 07:59 PDT, Pavel Feldman
no flags
Screenshot with Safari 4.0.3 and WebKit 48959 (36.47 KB, image/png)
2009-10-01 08:49 PDT, Mikhail Naganov
no flags
update patch (3.85 KB, patch)
2009-10-01 08:55 PDT, Mikhail Naganov
timothy: review-
removed substring matching (4.00 KB, patch)
2009-10-01 12:51 PDT, Mikhail Naganov
timothy: review+
timothy: commit-queue-
Mikhail Naganov
Comment 1 2009-10-01 07:44:46 PDT
Created attachment 40440 [details] propsed change
Timothy Hatcher
Comment 2 2009-10-01 07:50:55 PDT
Can you attach a screenshot?
Timothy Hatcher
Comment 3 2009-10-01 07:53:19 PDT
Comment on attachment 40440 [details] propsed change > + // Place "Close" button in docked mode on the same side as the window "Close" button in detached mode. > + var closeElement; > + if (InspectorController.platform().substr(0, 3) == "mac") { > + closeElement = document.getElementById("close-left"); > + } else { > + closeElement = document.getElementById("close-right"); > + } > + var closeButton = document.createElement("button"); > + closeButton.id = "close-button"; > + closeButton.addEventListener("click", this.close, true); > + closeElement.appendChild(closeButton); You can do more this in CSS. Using the .platfor-mac and .platform-win classes. So attach the listener to both elements and let CSS decided what one to show and hide. And have the button in both.
Pavel Feldman
Comment 4 2009-10-01 07:59:59 PDT
Created attachment 40441 [details] [IMAGE] close to the right (don't pay attention to shifted search field - i have an old and strange WebKit build)
Mikhail Naganov
Comment 5 2009-10-01 08:49:38 PDT
Created attachment 40446 [details] Screenshot with Safari 4.0.3 and WebKit 48959 I've implemented button positioning using CSS. Here is a screenshot of the latest Safari + WebKit with the patch that I'll attach in a few minutes.
Mikhail Naganov
Comment 6 2009-10-01 08:55:56 PDT
Created attachment 40447 [details] update patch
Timothy Hatcher
Comment 7 2009-10-01 09:06:17 PDT
Comment on attachment 40447 [details] update patch > +body.attached.platform-qt .toolbar-item[class*="close-"] { It would be better and faster for the engine to write this as: body.attached.platform-qt .toolbar-item.close-right, body.attached.platform-qt .toolbar-item.close-left Also more correct if we ever add somthing like "enclose-foo". > +body[class*="platform-mac"] .toolbar-item.close-right { Should be: body.platform-mac .toolbar-item.close-right > +body:not([class*="platform-mac"]) .toolbar-item.close-left { Should be: body:not(.platform-mac) .toolbar-item.close-left
Mikhail Naganov
Comment 8 2009-10-01 12:51:41 PDT
Created attachment 40471 [details] removed substring matching For mac Inspector actually has "mac-tiger" and "mac-leopard", so my initial intent was to merge these two cases into one (and also accept future possibilities, like "mac-snow-leopard"). But if you insist on not using substring matches, here is a plain version. Doing a negation on more than one class selector isn't intuitive. I've done it according to this W3C test: http://www.w3.org/Style/CSS/Test/CSS3/Selectors/current/html/full/flat/css3-modsel-14e.html I've checked that it works on Windows, and that after swapping these two :not's button isn't get hidden, too.
Pavel Feldman
Comment 9 2009-10-05 09:03:28 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/inspector/front-end/inspector.css M WebCore/inspector/front-end/inspector.html M WebCore/inspector/front-end/inspector.js Committed r49097
Note You need to log in before you can comment on or make changes to this bug.