Hi all,
I've found some obviously broken HTML tags on tbl_structure.php and I assume that this is due to the recent refactoring of this page. I couldn't find where the issue is in the code, but it manifests itself in the "more" dropdown as links there are now not clickable. Screenshot attached.
Another thing that I noticed in the new *_structure code are the new function names. Not sure if it's just me, but after a 2 minute look I found all of these functions where IMO the names can be improved:
PMA_getHtmlForPrintViewAndDataDictionaryLinks - too long? PMA_SortableTableHeader - starts with a capital unlike others PMA_getAliasAndTruename - incorrect camel case PMA_getStuffForEnginetable - meaningless + incorrect camel case PMA_getHtmlForCheckAlltableColumn - incorrect camel case PMA_getHtmlForSomeLinks - meaningless PMA_getHtmlForRowStatstableRow - incorrect camel case getHtmlForRowStatsTable - no PMA_ prefix PMA_getHtmlForDistincValueAction - spelling: "Distinct" PMA_getHtmlForActionsIntableStructure - incorrect camel case
Bye, Rouslan
On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella rouslan@placella.comwrote:
Hi all,
I've found some obviously broken HTML tags on tbl_structure.php and I assume that this is due to the recent refactoring of this page. I couldn't find where the issue is in the code, but it manifests itself in the "more" dropdown as links there are now not clickable. Screenshot attached.
Another thing that I noticed in the new *_structure code are the new function names. Not sure if it's just me, but after a 2 minute look I found all of these functions where IMO the names can be improved:
PMA_**getHtmlForPrintViewAndDataDict**ionaryLinks - too long? PMA_SortableTableHeader - starts with a capital unlike others PMA_getAliasAndTruename - incorrect camel case PMA_getStuffForEnginetable - meaningless + incorrect camel case PMA_**getHtmlForCheckAlltableColumn - incorrect camel case PMA_getHtmlForSomeLinks - meaningless PMA_getHtmlForRowStatstableRow - incorrect camel case getHtmlForRowStatsTable - no PMA_ prefix PMA_**getHtmlForDistincValueAction - spelling: "Distinct" PMA_**getHtmlForActionsIntableStruct**ure - incorrect camel case
Bye, Rouslan
Hi Rouslan, Thank you for spent time to look at my code. And I'll improve those function names, And I'll look at the mentioned bug also.
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com mailto:rouslan@placella.com> wrote:
Hi all, I've found some obviously broken HTML tags on tbl_structure.php and I assume that this is due to the recent refactoring of this page. I couldn't find where the issue is in the code, but it manifests itself in the "more" dropdown as links there are now not clickable. Screenshot attached. Another thing that I noticed in the new *_structure code are the new function names. Not sure if it's just me, but after a 2 minute look I found all of these functions where IMO the names can be improved: PMA___getHtmlForPrintViewAndDataDict__ionaryLinks - too long? PMA_SortableTableHeader - starts with a capital unlike others PMA_getAliasAndTruename - incorrect camel case PMA_getStuffForEnginetable - meaningless + incorrect camel case PMA___getHtmlForCheckAlltableColumn - incorrect camel case PMA_getHtmlForSomeLinks - meaningless PMA_getHtmlForRowStatstableRow - incorrect camel case getHtmlForRowStatsTable - no PMA_ prefix PMA___getHtmlForDistincValueAction - spelling: "Distinct" PMA___getHtmlForActionsIntableStruct__ure - incorrect camel case Bye, RouslanHi Rouslan, Thank you for spent time to look at my code. And I'll improve those function names, And I'll look at the mentioned bug also.
The bug is an unclosed div tag at line 1831 in structure.lib.php
Bye, Rouslan
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella rouslan@placella.comwrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com mailto:rouslan@placella.com> wrote:
Hi all, I've found some obviously broken HTML tags on tbl_structure.php and I assume that this is due to the recent refactoring of this page. I couldn't find where the issue is in the code, but it manifests itself in the "more" dropdown as links there are now not clickable. Screenshot attached. Another thing that I noticed in the new *_structure code are the new function names. Not sure if it's just me, but after a 2 minute look I found all of these functions where IMO the names can be improved: PMA___getHtmlForPrintViewAndDataDict__ionaryLinks - too long? PMA_SortableTableHeader - starts with a capital unlike others PMA_getAliasAndTruename - incorrect camel case PMA_getStuffForEnginetable - meaningless + incorrect camel case PMA___getHtmlForCheckAlltableColumn - incorrect camel case PMA_getHtmlForSomeLinks - meaningless PMA_getHtmlForRowStatstableRow - incorrect camel case getHtmlForRowStatsTable - no PMA_ prefix PMA___getHtmlForDistincValueAction - spelling: "Distinct" PMA___getHtmlForActionsIntableStruct__ure - incorrect camel case Bye, RouslanHi Rouslan, Thank you for spent time to look at my code. And I'll improve those function names, And I'll look at the mentioned bug also.
The bug is an unclosed div tag at line 1831 in structure.lib.php
Thanks Rouslan.
On 17/08/2012 13:17, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella <rouslan@placella.com mailto:rouslan@placella.com> wrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote: > > > On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com <mailto:rouslan@placella.com> > <mailto:rouslan@placella.com <mailto:rouslan@placella.com>>> wrote: > > Hi all, > > I've found some obviously broken HTML tags on tbl_structure.php and > I assume that this is due to the recent refactoring of this page. I > couldn't find where the issue is in the code, but it manifests > itself in the "more" dropdown as links there are now not clickable. > Screenshot attached. > > Another thing that I noticed in the new *_structure code are the new > function names. Not sure if it's just me, but after a 2 minute look > I found all of these functions where IMO the names can be improved: > > PMA___getHtmlForPrintViewAndDataDict__ionaryLinks - too long? > PMA_SortableTableHeader - starts with a capital unlike others > PMA_getAliasAndTruename - incorrect camel case > PMA_getStuffForEnginetable - meaningless + incorrect camel case > PMA___getHtmlForCheckAlltableColumn - incorrect camel case > PMA_getHtmlForSomeLinks - meaningless > PMA_getHtmlForRowStatstableRow - incorrect camel case > getHtmlForRowStatsTable - no PMA_ prefix > PMA___getHtmlForDistincValueAction - spelling: "Distinct" > PMA___getHtmlForActionsIntableStruct__ure - incorrect camel case > > Bye, > Rouslan > > > Hi Rouslan, > Thank you for spent time to look at my code. And I'll improve those > function names, > And I'll look at the mentioned bug also. The bug is an unclosed div tag at line 1831 in structure.lib.phpThanks Rouslan.
Hi Thilina,
There seems to be a whole bunch of other bugs on the table structure page that have to do with the dropdown menu. It would be great if you could look at this asap as these have caused breakages in my branch.
To reproduce, create a table with the following definition:
CREATE TABLE IF NOT EXISTS `gfdfgd` ( `a` int(11) NOT NULL, `s` linestring NOT NULL, `f` text NOT NULL, PRIMARY KEY (`a`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 ROW_FORMAT=COMPACT;
Then, please look at the attached screenshots. In screenshot-1, the fulltext link is missing from the text column. In screenshot-2, both the fulltext and spatial links are missing. In screenshot-3, you can see an unwanted link in the top left and a fulltext link for a spatial column. In screenshot-4, you can see an spatial link for a text column.
I didn't do a bisect...
Bye, Rouslan
On Fri, Aug 17, 2012 at 6:35 PM, Rouslan Placella rouslan@placella.comwrote:
On 17/08/2012 13:17, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella <rouslan@placella.com mailto:rouslan@placella.com> wrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote: > > > On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com <mailto:rouslan@placella.com> > <mailto:rouslan@placella.com <mailto:rouslan@placella.com>>**>wrote: > > Hi all, > > I've found some obviously broken HTML tags on tbl_structure.php and > I assume that this is due to the recent refactoring of this page. I > couldn't find where the issue is in the code, but it manifests > itself in the "more" dropdown as links there are now not clickable. > Screenshot attached. > > Another thing that I noticed in the new *_structure code are the new > function names. Not sure if it's just me, but after a 2 minute look > I found all of these functions where IMO the names can be improved: > > PMA___**getHtmlForPrintViewAndDataDict**__ionaryLinks - too long? > PMA_SortableTableHeader - starts with a capital unlike others > PMA_getAliasAndTruename - incorrect camel case > PMA_getStuffForEnginetable - meaningless + incorrect camel case > PMA___**getHtmlForCheckAlltableColumn - incorrect camel case > PMA_getHtmlForSomeLinks - meaningless > PMA_getHtmlForRowStatstableRow - incorrect camel case > getHtmlForRowStatsTable - no PMA_ prefix > PMA___**getHtmlForDistincValueAction - spelling: "Distinct" > PMA___**getHtmlForActionsIntableStruct**__ure - incorrect camel case > > Bye, > Rouslan > > > Hi Rouslan, > Thank you for spent time to look at my code. And I'll improve those > function names, > And I'll look at the mentioned bug also.
The bug is an unclosed div tag at line 1831 in structure.lib.phpThanks Rouslan.
Hi Thilina,
There seems to be a whole bunch of other bugs on the table structure page that have to do with the dropdown menu. It would be great if you could look at this asap as these have caused breakages in my branch.
To reproduce, create a table with the following definition:
CREATE TABLE IF NOT EXISTS `gfdfgd` ( `a` int(11) NOT NULL, `s` linestring NOT NULL, `f` text NOT NULL, PRIMARY KEY (`a`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 ROW_FORMAT=COMPACT;
Then, please look at the attached screenshots. In screenshot-1, the fulltext link is missing from the text column. In screenshot-2, both the fulltext and spatial links are missing. In screenshot-3, you can see an unwanted link in the top left and a fulltext link for a spatial column. In screenshot-4, you can see an spatial link for a text column.
I didn't do a bisect...
Thank you very much Rouslan. I'll look at those issues.
Thilina Buddika Abeyrathna a écrit :
On Fri, Aug 17, 2012 at 6:35 PM, Rouslan Placella rouslan@placella.comwrote:
On 17/08/2012 13:17, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella <rouslan@placella.com mailto:rouslan@placella.com> wrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote: > > > On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com <mailto:rouslan@placella.com> > <mailto:rouslan@placella.com <mailto:rouslan@placella.com>>**>wrote: > > Hi all, > > I've found some obviously broken HTML tags on tbl_structure.php and > I assume that this is due to the recent refactoring of this page. I > couldn't find where the issue is in the code, but it manifests > itself in the "more" dropdown as links there are now not clickable. > Screenshot attached. > > Another thing that I noticed in the new *_structure code are the new > function names. Not sure if it's just me, but after a 2 minute look > I found all of these functions where IMO the names can be improved: > > PMA___**getHtmlForPrintViewAndDataDict**__ionaryLinks - too long? > PMA_SortableTableHeader - starts with a capital unlike others > PMA_getAliasAndTruename - incorrect camel case > PMA_getStuffForEnginetable - meaningless + incorrect camel case > PMA___**getHtmlForCheckAlltableColumn - incorrect camel case > PMA_getHtmlForSomeLinks - meaningless > PMA_getHtmlForRowStatstableRow - incorrect camel case > getHtmlForRowStatsTable - no PMA_ prefix > PMA___**getHtmlForDistincValueAction - spelling: "Distinct" > PMA___**getHtmlForActionsIntableStruct**__ure - incorrect camel case > > Bye, > Rouslan > > > Hi Rouslan, > Thank you for spent time to look at my code. And I'll improve those > function names, > And I'll look at the mentioned bug also.
The bug is an unclosed div tag at line 1831 in structure.lib.phpThanks Rouslan.
Hi Thilina,
There seems to be a whole bunch of other bugs on the table structure page that have to do with the dropdown menu. It would be great if you could look at this asap as these have caused breakages in my branch.
To reproduce, create a table with the following definition:
CREATE TABLE IF NOT EXISTS `gfdfgd` ( `a` int(11) NOT NULL, `s` linestring NOT NULL, `f` text NOT NULL, PRIMARY KEY (`a`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 ROW_FORMAT=COMPACT;
Then, please look at the attached screenshots. In screenshot-1, the fulltext link is missing from the text column. In screenshot-2, both the fulltext and spatial links are missing. In screenshot-3, you can see an unwanted link in the top left and a fulltext link for a spatial column. In screenshot-4, you can see an spatial link for a text column.
I didn't do a bisect...
Thank you very much Rouslan. I'll look at those issues.
I see that Thilina has fixed most of these problems, except that the More drop-down is missing some of the links and has incorrect other links (for example, a linestring column should not have a link to add a FULLTEXT index).
On Sat, Aug 18, 2012 at 1:08 AM, Marc Delisle marc@infomarc.info wrote:
Thilina Buddika Abeyrathna a écrit :
On Fri, Aug 17, 2012 at 6:35 PM, Rouslan Placella <rouslan@placella.com wrote:
On 17/08/2012 13:17, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella <
rouslan@placella.com
mailto:rouslan@placella.com> wrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote: > > > On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com <mailto:rouslan@placella.com> > <mailto:rouslan@placella.com <mailto:rouslan@placella.com>>**>wrote: > > Hi all, > > I've found some obviously broken HTML tags on tbl_structure.php and > I assume that this is due to the recent refactoring of this page. I > couldn't find where the issue is in the code, but it
manifests
> itself in the "more" dropdown as links there are now not clickable. > Screenshot attached. > > Another thing that I noticed in the new *_structure code are the new > function names. Not sure if it's just me, but after a 2 minute look > I found all of these functions where IMO the names can be improved: > > PMA___**getHtmlForPrintViewAndDataDict**__ionaryLinks - toolong? > PMA_SortableTableHeader - starts with a capital unlike
others
> PMA_getAliasAndTruename - incorrect camel case > PMA_getStuffForEnginetable - meaningless + incorrect camelcase
> PMA___**getHtmlForCheckAlltableColumn - incorrect camel case > PMA_getHtmlForSomeLinks - meaningless > PMA_getHtmlForRowStatstableRow - incorrect camel case > getHtmlForRowStatsTable - no PMA_ prefix > PMA___**getHtmlForDistincValueAction - spelling: "Distinct" > PMA___**getHtmlForActionsIntableStruct**__ure - incorrectcamel case > > Bye, > Rouslan > > > Hi Rouslan, > Thank you for spent time to look at my code. And I'll improve
those
> function names, > And I'll look at the mentioned bug also. The bug is an unclosed div tag at line 1831 in structure.lib.phpThanks Rouslan.
Hi Thilina,
There seems to be a whole bunch of other bugs on the table structure
page
that have to do with the dropdown menu. It would be great if you could
look
at this asap as these have caused breakages in my branch.
To reproduce, create a table with the following definition:
CREATE TABLE IF NOT EXISTS `gfdfgd` ( `a` int(11) NOT NULL, `s` linestring NOT NULL, `f` text NOT NULL, PRIMARY KEY (`a`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 ROW_FORMAT=COMPACT;
Then, please look at the attached screenshots. In screenshot-1, the fulltext link is missing from the text column. In screenshot-2, both the fulltext and spatial links are missing. In screenshot-3, you can see an unwanted link in the top left and a fulltext link for a spatial column. In screenshot-4, you can see an spatial link for a text column.
I didn't do a bisect...
Thank you very much Rouslan. I'll look at those issues.
I see that Thilina has fixed most of these problems, except that the More drop-down is missing some of the links and has incorrect other links (for example, a linestring column should not have a link to add a FULLTEXT index).
Fixed in cd2177ec4b0fa58be20ca563cc7bcbd645126b93 commit.
Le 2012-08-17 22:34, Thilina Buddika Abeyrathna a écrit :
On Sat, Aug 18, 2012 at 1:08 AM, Marc Delisle marc@infomarc.info wrote:
Thilina Buddika Abeyrathna a écrit :
On Fri, Aug 17, 2012 at 6:35 PM, Rouslan Placella <rouslan@placella.com wrote:
On 17/08/2012 13:17, Thilina Buddika Abeyrathna wrote:
On Fri, Aug 17, 2012 at 4:31 PM, Rouslan Placella <
rouslan@placella.com
mailto:rouslan@placella.com> wrote:
On 17/08/2012 12:00, Thilina Buddika Abeyrathna wrote: > > > On Fri, Aug 17, 2012 at 1:42 PM, Rouslan Placella <rouslan@placella.com <mailto:rouslan@placella.com> > <mailto:rouslan@placella.com <mailto:rouslan@placella.com>>**>wrote: > > Hi all, > > I've found some obviously broken HTML tags on tbl_structure.php and > I assume that this is due to the recent refactoring of this page. I > couldn't find where the issue is in the code, but it
manifests
> itself in the "more" dropdown as links there are now not clickable. > Screenshot attached. > > Another thing that I noticed in the new *_structure code are the new > function names. Not sure if it's just me, but after a 2 minute look > I found all of these functions where IMO the names can be improved: > > PMA___**getHtmlForPrintViewAndDataDict**__ionaryLinks - toolong? > PMA_SortableTableHeader - starts with a capital unlike
others
> PMA_getAliasAndTruename - incorrect camel case > PMA_getStuffForEnginetable - meaningless + incorrect camelcase
> PMA___**getHtmlForCheckAlltableColumn - incorrect camel case > PMA_getHtmlForSomeLinks - meaningless > PMA_getHtmlForRowStatstableRow - incorrect camel case > getHtmlForRowStatsTable - no PMA_ prefix > PMA___**getHtmlForDistincValueAction - spelling: "Distinct" > PMA___**getHtmlForActionsIntableStruct**__ure - incorrectcamel case > > Bye, > Rouslan > > > Hi Rouslan, > Thank you for spent time to look at my code. And I'll improve
those
> function names, > And I'll look at the mentioned bug also. The bug is an unclosed div tag at line 1831 in structure.lib.phpThanks Rouslan.
Hi Thilina,
There seems to be a whole bunch of other bugs on the table structure
page
that have to do with the dropdown menu. It would be great if you could
look
at this asap as these have caused breakages in my branch.
To reproduce, create a table with the following definition:
CREATE TABLE IF NOT EXISTS `gfdfgd` ( `a` int(11) NOT NULL, `s` linestring NOT NULL, `f` text NOT NULL, PRIMARY KEY (`a`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 ROW_FORMAT=COMPACT;
Then, please look at the attached screenshots. In screenshot-1, the fulltext link is missing from the text column. In screenshot-2, both the fulltext and spatial links are missing. In screenshot-3, you can see an unwanted link in the top left and a fulltext link for a spatial column. In screenshot-4, you can see an spatial link for a text column.
I didn't do a bisect...
Thank you very much Rouslan. I'll look at those issues.
I see that Thilina has fixed most of these problems, except that the More drop-down is missing some of the links and has incorrect other links (for example, a linestring column should not have a link to add a FULLTEXT index).
Fixed in cd2177ec4b0fa58be20ca563cc7bcbd645126b93 commit.
Looks fine. Pushed to upstream, thanks.
-
Marc Delisle -
Rouslan Placella -
Thilina Buddika Abeyrathna