Closed
Bug 590476
Opened 14 years ago
Closed 14 years ago
Open search engines awesome screen row take a lot of space
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(fennec2.0+)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0+ | --- |
People
(Reporter: vingtetun, Assigned: vingtetun)
References
()
Details
Attachments
(4 files, 3 obsolete files)
60.73 KB,
image/png
|
Details | |
68.71 KB,
image/png
|
Details | |
16.82 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
2.65 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
The patch just move the open search engines to a popup that you can see by clicking on the favicon button on the top left side (when the awesome screen is dislayed).
This is just a list that search the current terms entered into the urlbar for now (it does not switch the "All Pages" view)
For an idea of the look: see screenshots
Assignee | ||
Comment 1•14 years ago
|
||
Assignee | ||
Comment 2•14 years ago
|
||
Comment 3•14 years ago
|
||
This bug is part of Fennec 2 work to maximize the awesomescreen, especially when using a virtual keyboard.
Comment 4•14 years ago
|
||
added link to design page
Assignee | ||
Comment 5•14 years ago
|
||
This patch depends on bug 590779 for the urlbar-state observer
Attachment #469010 -
Attachment is obsolete: true
Attachment #469444 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 6•14 years ago
|
||
Patch rebased on the new patch in bug 590779
Assignee: nobody → 21
Attachment #469444 -
Attachment is obsolete: true
Attachment #469470 -
Flags: review?(mark.finkle)
Attachment #469444 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 7•14 years ago
|
||
Sorry for the spam the previous patch was a failure.
Attachment #469470 -
Attachment is obsolete: true
Attachment #469476 -
Flags: review?(mark.finkle)
Attachment #469470 -
Flags: review?(mark.finkle)
Updated•14 years ago
|
tracking-fennec: --- → 2.0+
Comment 8•14 years ago
|
||
Comment on attachment 469476 [details] [diff] [review]
Patch v0.3
> handleIdentityButtonEvent: function(aEvent) {
> aEvent.stopPropagation();
>
>+ if (Elements.urlbarState.getAttribute("mode") == "edit") {
>+ CommandUpdater.doCommand("cmd_opensearch");
>+ return;
>+ }
>+
Add a comment as to why "edit" matters
>+ <box id="urlbar-image-box" mousethrough="always" observes="bcast_urlbarState">
> <image id="urlbar-throbber"/>
>+ <image id="urlbar-magnifier"/>
> <image id="urlbar-favicon" hidden="true"/>
Should urlbar-magnifier be hidden="true" too?
> <vbox id="menulist-popup" class="dialog-dark">
>+ <label id="menulist-title" crop="center"/>
I think you need a flex="1" here to make the crop="center" work
>+#urlbar-magnifier {
>+ list-style-image: url("chrome://browser/skin/images/navigation-magnifier-30.png");
>+ display: none;
Ah, I see you have it here. I wonder if having hidden=true" in the XUL has any Ts impact?
r+ with the nits answered
Attachment #469476 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 9•14 years ago
|
||
(In reply to comment #8)
> >+ <box id="urlbar-image-box" mousethrough="always" observes="bcast_urlbarState">
> > <image id="urlbar-throbber"/>
> >+ <image id="urlbar-magnifier"/>
> > <image id="urlbar-favicon" hidden="true"/>
>
> Should urlbar-magnifier be hidden="true" too?
>
> >+#urlbar-magnifier {
> >+ list-style-image: url("chrome://browser/skin/images/navigation-magnifier-30.png");
> >+ display: none;
>
> Ah, I see you have it here. I wonder if having hidden=true" in the XUL has any
> Ts impact?
>
> r+ with the nits answered
I guess hidden=true vs display: none has the same impact and I don't think this has a big impact for this specific case
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#36
Assignee | ||
Comment 10•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•14 years ago
|
||
I've introduce a crazy string! Sorry for that.
Attachment #469690 -
Flags: review?(mbrubeck)
Updated•14 years ago
|
Attachment #469690 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11)
> Created attachment 469690 [details] [diff] [review]
> bustage-fix
>
> I've introduce a crazy string! Sorry for that.
http://hg.mozilla.org/mobile-browser/rev/24af494a34e4
(Thanks for the quick review Matt)
Updated•14 years ago
|
Flags: in-testsuite?
Flags: in-litmus?
Comment 13•14 years ago
|
||
verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b5pre) Gecko/20100830 Namoroka/4.0b5pre Fennec/2.0a1pre
and
Mozilla/5.0 (Android; Linux armv71; Nokia N900; en-US; rv:2.0b5pre) Gecko/20100830 Namoroka/4.0b5pre Fennec/2.0a1pre
Status: RESOLVED → VERIFIED
Comment 14•14 years ago
|
||
litmus testcases updated/added to regression test this bug:
https://litmus.mozilla.org/show_test.cgi?id=12141
https://litmus.mozilla.org/show_test.cgi?id=12169
https://litmus.mozilla.org/show_test.cgi?id=12248
https://litmus.mozilla.org/show_test.cgi?id=12245
https://litmus.mozilla.org/show_test.cgi?id=12185
https://litmus.mozilla.org/show_test.cgi?id=12122
https://litmus.mozilla.org/show_test.cgi?id=12695
Flags: in-litmus? → in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•