Bug 75471 - [Qt] Add UI for JavaScript Alert dialog in the Qt MiniBrowser
Summary: [Qt] Add UI for JavaScript Alert dialog in the Qt MiniBrowser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexander Færøy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-03 06:53 PST by Alexander Færøy
Modified: 2012-01-03 08:20 PST (History)
3 users (show)

See Also:

Attachments
Patch (8.82 KB, patch)
2012-01-03 06:58 PST, Alexander Færøy 		no flags Details | Formatted Diff | Diff
Patch (9.46 KB, patch)
2012-01-03 07:39 PST, Alexander Færøy 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Færøy 2012-01-03 06:53:05 PST 
SSIA.
Comment 1 Alexander Færøy 2012-01-03 06:58:15 PST 
Created attachment 120941 [details]
Patch
Comment 2 Alexander Færøy 2012-01-03 06:58:41 PST 
Adding Tor Arne.
Comment 3 Tor Arne Vestbø 2012-01-03 07:08:05 PST 
Comment on attachment 120941 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120941&action=review

Looks good, but a few comments before landing

> Tools/MiniBrowser/qt/qml/Dialog.qml:58
> +        clip: true

Do we need to clip it?

> Tools/MiniBrowser/qt/qml/Dialog.qml:64
> +            font.weight: Font.Bold

Width, and eliding, so that we don't have to clip?

> Tools/MiniBrowser/qt/qml/Dialog.qml:69
> +            anchors.centerIn: parent

Should this have a width and a wrap mode?

> Tools/MiniBrowser/qt/qml/Dialog.qml:76
> +            anchors.margins: 10
> +            anchors.bottom: staticContent.bottom
> +            anchors.horizontalCenter: staticContent.horizontalCenter

I prefer anchors {} when there are more than one property

> Tools/MiniBrowser/qt/qml/DialogButton.qml:37
> +    color: "#fbfbfb"

color: mouseArea.pressed ? ....

> Tools/MiniBrowser/qt/qml/DialogButton.qml:57
> +        onPressed: button.color = "#eaeaea"

You can remove this and use a binding (above)
Comment 4 Michael Brüning 2012-01-03 07:12:02 PST 
I think we should also include the new qml files in the OTHER_FILES section in MiniBrowser.pro ...
Comment 5 Alexander Færøy 2012-01-03 07:39:23 PST 
Created attachment 120944 [details]
Patch
Comment 6 WebKit Review Bot 2012-01-03 08:20:46 PST 
Comment on attachment 120944 [details]
Patch

Clearing flags on attachment: 120944

Committed r103940: <http://trac.webkit.org/changeset/103940>
Comment 7 WebKit Review Bot 2012-01-03 08:20:50 PST 
All reviewed patches have been landed.  Closing bug.